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.

Sign up for GitHub

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

Jump to bottom

Throw when writeFragment cannot identify object #6859

Merged
merged 4 commits into from
Aug 21, 2020
Merged

Throw when writeFragment cannot identify object #6859

merged 4 commits into from
Aug 21, 2020

Conversation

jcreighton
Copy link
Contributor

As noted in #6706, writeFragment can fail to identify the object and return undefined. This can be a pain to debug so we now warn when that's the case.

Checklist:

  • If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
  • Make sure all of the significant new logic is covered by tests

@jcreighton jcreighton requested a review from benjamn August 18, 2020 20:39
benjamn
benjamn reviewed Aug 19, 2020
View reviewed changes
Copy link
Member

@benjamn benjamn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After giving it some more thought, I think this change could end up removing code, even though it's adding functionality. Ideas below!

src/cache/inmemory/writeToStore.ts Outdated Show resolved Hide resolved
src/cache/inmemory/writeToStore.ts Outdated Show resolved Hide resolved
@benjamn benjamn added this to the Post 3.0 milestone Aug 19, 2020
benjamn added a commit that referenced this pull request Aug 20, 2020
@benjamn
Implement useReactiveVar hook.
2f494c8 
Closes #6859.
benjamn
benjamn approved these changes Aug 21, 2020
View reviewed changes
Copy link
Member

@benjamn benjamn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with one last suggestion!

src/cache/inmemory/writeToStore.ts Outdated Show resolved Hide resolved
benjamn added a commit that referenced this pull request Aug 21, 2020
@benjamn
Mention PR #6859 in CHANGELOG.md.
9db2730
@benjamn benjamn force-pushed the writeFragment-identity-error branch from 9db2730 to ea5a356 Compare August 21, 2020 18:51
@benjamn benjamn changed the base branch from master to release-3.2 August 21, 2020 18:52
@benjamn
Copy link
Member

benjamn commented Aug 21, 2020

Since this PR introduces a new exception throwing behavior, I think it needs to be considered a minor breaking change, so I rebased/retargeted it to release-3.2, jfyi.

@benjamn benjamn merged commit 766d3ff into release-3.2 Aug 21, 2020
@benjamn benjamn mentioned this pull request Aug 21, 2020
Merged
11 tasks
@jcreighton jcreighton deleted the writeFragment-identity-error branch April 2, 2021 21:06
@jcreighton jcreighton restored the writeFragment-identity-error branch April 2, 2021 21:06
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@benjamn benjamn benjamn approved these changes

@StephenBarlow StephenBarlow Awaiting requested review from StephenBarlow

Assignees

@jcreighton jcreighton

Labels
🧞‍♂️ enhancement
Projects
None yet
Milestone
Post 3.0
Development

Successfully merging this pull request may close these issues.
2 participants
@jcreighton @benjamn