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(graphcache): Mutable links on InMemoryData store #3516

Merged
merged 2 commits into from
Mar 2, 2024
Merged

Conversation

kitten
Copy link
Member

@kitten kitten commented Mar 2, 2024

See related PR: #3503

Summary

When cache.resolve returns a list of entity links, i.e. an array link, a user may be tempted to mutate this list before writing it back to the store.

This can have unforeseen consequences if:

  • cache.link then doesn't write to the same layer
  • cache.link isn't called at all
  • the array is completely changed and then read back in an unrelated operation

This is because, unlike cache.link, cache.resolve doesn't make a copy of links before returning them. This means that mutating a link array leads to direct modifications to the internal caching layer.

Set of changes

  • Call ensureLink before returning the links via cache.resolve to copy them

@kitten kitten requested a review from JoviDeCroock March 2, 2024 13:25
Copy link

changeset-bot bot commented Mar 2, 2024

🦋 Changeset detected

Latest commit: 38d3194

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@urql/exchange-graphcache Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@kitten kitten merged commit 969935c into main Mar 2, 2024
7 checks passed
@kitten kitten deleted the fix/mutable-links branch March 2, 2024 13:34
@github-actions github-actions bot mentioned this pull request Mar 2, 2024
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.

2 participants