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.

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(array): fix set() with a path going through an array #29

Merged
merged 1 commit into from
Mar 21, 2022

Conversation

tzi
Copy link
Contributor

@tzi tzi commented Mar 21, 2022

When we use set() with a path that contain an array, the array reconstruction can miss some elements.

For example, considering having this base object:

{
  a: {
    b: [{ id: 1, next: 1 }, { id: 2, next: 1 }, { id: 3, next: 1 }, { id: 4, next: 1 }],
  },
};

If we want to update the path a.b.0.next there was no problem, but if we update the path a.b.1.next we lost the thrid and forth elements.

Capture d’écran 2022-03-21 à 13 32 43

This happens when rebuilding the array. Here is an extract of the library code:

return [
  ...currentBase.slice(0, key),
  set(currentBase[key]),
   ...currentBase.slice(key + 1),
];

In our case, key + 1 is evaluted to 11 since key was a string. So we lost the values from 2 to 10.

My PR add the described test, and a parseInt() to avoid this problem.

@fdubost fdubost requested a review from flepretre March 21, 2022 13:24
@flepretre flepretre merged commit fe09785 into BedrockStreaming:master Mar 21, 2022
@tzi tzi deleted the fix-set-path branch March 21, 2022 15:55
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.

None yet

6 participants