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

Prevent immer from freeze operations. #4043

Conversation

ulion
Copy link
Contributor

@ulion ulion commented Jan 13, 2021

Is this adding or improving a feature or fixing a bug?

Fixing a bug.

What's the new behavior?

After copy (editor.getFragment), the editor.operations is not frozen (then no crash due to any update). if in case you actually using immer 7+ though Slate using v5 but if the client code has other dependencies which using higher version of immer, finally the Slate could actually calling immer 7+.

How does this change work?

Like https://github.com/ianstormtaylor/slate/pull/3850/files#r480954559 suggested, but without upgrade the immer version in package.json

Have you checked that...?

  • The new code matches the existing patterns and styles.
  • The tests pass with yarn test.
  • The linter passes with yarn lint. (Fix errors with yarn fix.)
  • The relevant examples still work. (Run examples with yarn start.)

Does this fix any issues or need any specific reviewers?

Fixes: #4034
Reviewers: @BrentFarese

@BrentFarese
Copy link
Collaborator

BrentFarese commented Jan 14, 2021

Is Immer freezing editor.operations caused by the bump in Immer in 60.1 from 5.0.0 to 5.3.6? Do you know @ulion? Also, I'm probably inclined to merge #3850 as this is a duplicate?

@BrentFarese BrentFarese self-assigned this Jan 14, 2021
@BrentFarese BrentFarese added this to the Slate 1.0 milestone Jan 14, 2021
@ulion
Copy link
Contributor Author

ulion commented Jan 14, 2021

Is Immer freezing editor.operations caused by the bump in Immer in 60.1 from 5.0.0 to 5.3.6? Do you know @ulion? Also, I'm probably inclined to merge #3850 as this is a duplicate?

should be immer 7.0 issue. since other packages in my project use immer 7.0, I think sometimes no matter which version slate using, it is effected.

@ulion
Copy link
Contributor Author

ulion commented Jan 14, 2021

well, I am not sure, since I only have a mixed immer 7 env. if you have pure 0.60.1 test env, simply copy then click anywhere will tell us whether it's effected.

@BrentFarese
Copy link
Collaborator

well, I am not sure, since I only have a mixed immer 7 env. if you have pure 0.60.1 test env, simply copy then click anywhere will tell us whether it's effected.

K. Going to close this as it's a duplicate of #3850 and will merge #3850 once the PR owner answers a question(s) I had. Will get this fix + the Immer version bump in though soon! Thanks.

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.

Error when deselecting or pasting: Cannot add property 0, object is not extensible
2 participants