Resolves Issue with DeSerialization of Date: #4696 #4706
Conversation
|
Sorry for the delay. Would you mind adding a test? |
|
@sorvell I have added the test case and updated the pull request |
| let ser = []; | ||
| ser[0] = window.XFoo.prototype._deserializeValue(dateArr[1], Date); | ||
| ser[1] = window.XFoo.prototype._deserializeValue(dateArr[2], Date); | ||
| ser[2] = window.XFoo.prototype._deserializeValue(dateArr[3], Date); |
There was a problem hiding this comment.
It seems like there is no assertion for this line? Could you add one for it? Also the variable name ser seems cryptic to me. Would be better to use a longer name to improve readability
There was a problem hiding this comment.
here I am just storing the return values of the deserializeValue function. I have used them in assert statements below.
I took this approach so that assert statements becomes more readable.
I shall change the variable name ser to serializedValue which shall become more readable.
There was a problem hiding this comment.
I don't disagree with the separation of the serializedValue array and the assertions, but I meant that I don't see any reference to ser[2] in the assertions on the lines below.
There was a problem hiding this comment.
Ping @ronak1009 could you rebase and update the variable names per this comment 😄
|
I manually updated the test suite in 86a64b6 |
Fixes #4696: Issue with DeSerialization of Date