fix(utils/looseEqual): update looseEqual to handle dates (close #7928) #7929
Conversation
| } else { | ||
| /* istanbul ignore next */ | ||
| return false | ||
| const isDateA = a instanceof Date |
There was a problem hiding this comment.
hmm, why did you added it this way instead of another else if condition?
There was a problem hiding this comment.
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.
|
I created #7940, this is what I meant by adding an else if. Tell me if I missed a scenario you thought about, I kept the test exactly as you added it |
|
Looks good to me, and no worries about the commit. Glad I could help locate the bug! |
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
If yes, please describe the impact and migration path for existing applications:
The PR fulfills these requirements:
devbranch for v2.x (or to a previous version branch), not themasterbranchfix #xxx[,#xxx], where "xxx" is the issue number)If adding a new feature, the PR's description includes:
Other information: