Skip to content

Conversation

@ellyxir
Copy link
Contributor

@ellyxir ellyxir commented Jul 16, 2025

Summary by cubic

Fixed issues in Gmail and RSS importers by correcting array path handling and improving document creation logic.

  • Bug Fixes
    • Fixed array path generation when setting array length to zero.
    • Improved detection of array keys to avoid NaN errors.
    • Updated logic to handle root writes with a value property.

ellyxir added 2 commits July 16, 2025 14:05
… detection in retry logic, cherrypick bernis change for handling for root writes with value property
@ellyxir ellyxir requested review from seefeldb and ubik2 July 16, 2025 21:34
@ellyxir ellyxir self-assigned this Jul 16, 2025
@linear
Copy link

linear bot commented Jul 16, 2025

@ellyxir
Copy link
Contributor Author

ellyxir commented Jul 16, 2025

reviewed with robin

@ellyxir ellyxir merged commit ba0cc05 into main Jul 16, 2025
7 checks passed
@ellyxir ellyxir deleted the ellyse/ct-580-gmail-and-rss-importers-have-failures-clean branch July 16, 2025 21:47
Copy link
Contributor

@seefeldb seefeldb left a 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;
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants