Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(utils/looseEqual): update looseEqual to handle dates (close #7928) #7929

Closed
wants to merge 1 commit into from
Closed

fix(utils/looseEqual): update looseEqual to handle dates (close #7928) #7929

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "vue",
"version": "2.5.16",
"version": "2.5.17-beta.0",
"description": "Reactive, component-oriented view layer for modern web interfaces.",
"main": "dist/vue.runtime.common.js",
"module": "dist/vue.runtime.esm.js",
Expand Down
22 changes: 14 additions & 8 deletions src/shared/util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,15 +282,21 @@ export function looseEqual (a: any, b: any): boolean {
return a.length === b.length && a.every((e, i) => {
return looseEqual(e, b[i])
})
} else if (!isArrayA && !isArrayB) {
const keysA = Object.keys(a)
const keysB = Object.keys(b)
return keysA.length === keysB.length && keysA.every(key => {
return looseEqual(a[key], b[key])
})
} else {
/* istanbul ignore next */
return false
const isDateA = a instanceof Date
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, why did you added it this way instead of another else if condition?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally defined the variables at the top below the isArrayA, and isArrayB variables, but that was calling instanceof for everything that got passed in and added about 500 milliseconds to the test run. It's my first time perusing the code base. Not sure how much this method is used and if it would have a significant impact on anything else.

Also, I originally called instanceof directly inside the if, but tried to follow the code style of putting them in variables first.

const isDateB = b instanceof Date
if (isDateA && isDateB) {
return a.toUTCString() === b.toUTCString()
} else if (!isArrayA && !isArrayB && !isDateA && !isDateB) {
const keysA = Object.keys(a)
const keysB = Object.keys(b)
return keysA.length === keysB.length && keysA.every(key => {
return looseEqual(a[key], b[key])
})
} else {
/* istanbul ignore next */
return false
}
}
} catch (e) {
/* istanbul ignore next */
Expand Down
27 changes: 27 additions & 0 deletions test/unit/features/directives/model-select.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -588,4 +588,31 @@ describe('Directive v-model select', () => {
}).then(done)
})
})

// #7928
it('should correctly handle option with date value', done => {
const vm = new Vue({
data: {
dates: [
new Date(1520000000000),
new Date(1522000000000),
new Date(1516000000000)
],
selectedDate: null
},
template:
'<div>' +
'<select v-model="selectedDate">' +
'<option v-for="(date, i) in dates" :key="i" :value="date">' +
'{{date}}' +
'</option>' +
'</select>' +
'</div>'
}).$mount()

vm.selectedDate = vm.dates[2]
waitForUpdate(() => {
expect(vm.$el.firstChild.selectedIndex).toBe(2)
}).then(done)
})
})