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

[relay-runtime] Unfreeze JSON object to allow reassignment #2051

Closed

Conversation

dbslone
Copy link

@dbslone dbslone commented Aug 22, 2017

Fixes #2049

The issue described in #2049 arises because the object being passed to recycleNodesInto is a frozen object. So nextObject[key] = nextValue will always fail.

This fix currently causes one of tests in packages/react-relay/classic/legacy/store/__tests__/recycleNodesInto-test.js to fail because it uses === comparison but the returned result is the same. Not sure if these tests are still being used since they don't appear to be included when running npm test from the root directory.

@leebyron
Copy link
Contributor

cc @kassens @josephsavona - can you help review this one? Looks like a real issue in Relay attempting to mutate a frozen record, however we should get a gut-check on whether this is the right fix.

@dbslone
Copy link
Author

dbslone commented Sep 28, 2017

@leebyron @kassens @josephsavona any updates on if this best strategy to resolve this issue?

@josephsavona
Copy link
Contributor

Is this still an issue on master? This has come up previously and we've already made several fixes to recycleNodesInto in order to avoid ending up mutating the previous input exactly because it could be frozen. I'm also concerned about the performance implications of this implementation - at a glance it would seem to incur quite a bit of copying items/properties.

@jardakotesovec
Copy link
Contributor

@josephsavona Just tried with 1.4.1 and also with master and still the same issue.

@mike-marcacci
Copy link
Contributor

mike-marcacci commented Nov 15, 2017

I have another fix for this in #2193 which takes a different approach.

@dbslone
Copy link
Author

dbslone commented Jan 29, 2018

Closing PR in favor of #2192

@dbslone dbslone closed this Jan 29, 2018
@dbslone dbslone deleted the relay-runtime-dethaw-json-scalar branch October 3, 2018 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Modern] bug when updating json scalar field
6 participants