-
Notifications
You must be signed in to change notification settings - Fork 9
Ellyse/ct 580 gmail and rss importers have failures clean #1424
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.
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
Ellyse/ct 580 gmail and rss importers have failures clean #1424
Conversation
… detection in retry logic, cherrypick bernis change for handling for root writes with value property
|
reviewed with robin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, good catch on the length!
| nextValue = | ||
| nextValue[key] = | ||
| (typeof Number(key) === "number" ? [] : {}) as typeof nextValue; | ||
| (!Number.isNaN(Number(key)) ? [] : {}) as typeof nextValue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interesting, is this for clarity or did this misbehave?
if we change to Number., then let's use isInteger instead, since array indices have to be that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Number(key) might return NaN, however, typeof NaN is "number", so therefore its always true and it will never create an object. i guess thats why I used NaN since its closer probably to the intended behaviour. still, isInteger is better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Number will also cast string|number, where isInteger will always fail for strings, not sure of key type here
Summary by cubic
Fixed issues in Gmail and RSS importers by correcting array path handling and improving document creation logic.