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

Bump immer from 7.0.7 to 8.0.1 #4050

Merged
merged 1 commit into from May 5, 2021

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Jan 21, 2021

Bumps immer from 7.0.7 to 8.0.1.

Release notes

Sourced from immer's releases.

v8.0.1

8.0.1 (2021-01-20)

Bug Fixes

v8.0.0

8.0.0 (2020-11-17)

feature

BREAKING CHANGES

  • always freeze by default, even in production mode. Use setAutoFreeze(process.env.NODE_ENV !== 'production') for the old behavior. See immerjs/immer#687 for the rationale. Fixes #649, #681, #687

v7.0.15

7.0.15 (2020-11-17)

Bug Fixes

v7.0.14

7.0.14 (2020-10-20)

Bug Fixes

v7.0.13

7.0.13 (2020-10-20)

Bug Fixes

  • reconcile if the original value is assigned after creating a draft. Fixes #659 (c0e6749)

v7.0.12

7.0.12 (2020-10-20)

Bug Fixes

... (truncated)

Commits
  • da2bd4f fix: Fixed security issue #738: prototype pollution possible when applying pa...
  • d75de70 chore: fix Buffer deprecation warning in test (#706)
  • 8fbf93c docs: Add referential equality to pitfalls (#731)
  • c21a2ef docs: Update current.md (#728)
  • 211314c docs: add cool-store into built-with.md (#724)
  • e8fd805 chore(tests): use UTC date string in tests to be timezone independent (#705)
  • fe8f589 chore(comments): update comments (#727)
  • d8121d6 chore(docs): Fix typo in pitfalls.md (#729)
  • 5379cdd chore(docs): Update example-reducer.md (#734)
  • d3908e1 chore(deps): bump dot-prop from 4.2.0 to 4.2.1 in /website (#735)
  • Additional commits viewable in compare view

Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
  • @dependabot use these labels will set the current labels as the default for future PRs for this repo and language
  • @dependabot use these reviewers will set the current reviewers as the default for future PRs for this repo and language
  • @dependabot use these assignees will set the current assignees as the default for future PRs for this repo and language
  • @dependabot use this milestone will set the current milestone as the default for future PRs for this repo and language

You can disable automated security fix PRs for this repo from the Security Alerts page.

@cprodescu
Copy link

cprodescu commented Feb 24, 2021

I don't think this is a trivial dependency update.

I've done this update in my project [1] using yarn resolutions, and I've noticed that the slate editor starts failing.
In my case you can repro by running these steps in order:

  • open an editor with at least 2 rows (even 2 empty rows)
  • select all (CMD + A)
  • copy (CMD + C)
  • arrow down

This will cause a crash with

Uncaught TypeError: Cannot delete property '2' of [object Array]
    at Array.pop (<anonymous>)
    at Object.e.apply (with-history.ts:92)
    at Object.setSelection (selection.ts:195)
    at Object.select (selection.ts:113)
    at HTMLDocument.<anonymous> (editable.tsx:423)

[1]

    "slate": "^0.59.0",
    "slate-history": "^0.59.0",
    "slate-hyperscript": "^0.59.0",
    "slate-react": "^0.59.0",

@druid8
Copy link

druid8 commented Mar 2, 2021

I confirm, editor stopped working after upgrade to 8.0.1.
The issue is (in this case) in Node.fragment() method which uses immer produce to create fragment. It seems that now immer makes all properties of base object as non-configurable which makes children and history objects immutable thus no changes can be made in editor state anymore.

For me it looks as there is misusage of immer at all. Immer is to "mutate" immutable states, editor instance is not immutable state, so we can't blame immer that it makes base object immutable as a side effect as only immutable objects should be passed as base object to produce.

This bug can be reproduced in more ways. Any method which uses produce() and accepts editor instance as an argument makes editor immutable and than any operation which tries to mutate editor state cause crash.

@cabbiepete
Copy link

cabbiepete commented Mar 28, 2021

Making a note here that immer <8.0.1 suffers from a Prototype Pollution high severity vulnerability according to npm audit https://npmjs.com/advisories/1603 this means Slate JS adds this as it depends on immer 7.x

Having a high severity caused by including slate vulnerability could impact the adoption of slate.

There is not an obvious solve except to move away from immer given @druid8's comment above

@williamstein
Copy link

There is not an obvious solve except to move away from immer given @druid8's comment above

Is it possible to fork immer 7.x and backport the fix?

@ianstormtaylor
Copy link
Owner

/rebase

@github-actions github-actions bot force-pushed the dependabot/npm_and_yarn/immer-8.0.1 branch from 9eb8dcc to 15d0527 Compare March 31, 2021 18:23
@changeset-bot
Copy link

changeset-bot bot commented Mar 31, 2021

⚠️ No Changeset found

Latest commit: b7c93bb

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@dependabot dependabot bot changed the base branch from master to main April 1, 2021 17:06
@aosq
Copy link

aosq commented Apr 2, 2021

If setAutoFreeze was set to false globally for Slate with the upgrade to Immer 8.0.1, would that require any changes?

There's some discussion about the reasoning and performance implications of auto freezing (or not auto freezing) in immerjs/immer#687.

@TheSpyder
Copy link
Collaborator

I confirm, editor stopped working after upgrade to 8.0.1.
The issue is (in this case) in Node.fragment() method which uses immer produce to create fragment. It seems that now immer makes all properties of base object as non-configurable which makes children and history objects immutable thus no changes can be made in editor state anymore.

This should have been fixed in #3850 (https://github.com/ianstormtaylor/slate/pull/3850/files#diff-36e80a658eb32e4912dc75f66345c365d4c599d9de5b32b633f441b03f571575) with the upgrade to immer 7.

I have also forcibly upgraded my copy of Slate to 8.0.1, and have had no issues with Editor.fragment() (which uses Node.fragment()). I'm not using slate-react, though, only the core.

@ianstormtaylor
Copy link
Owner

If anyone can confirm what @TheSpyder said ^ above for slate-react I'd be happy to merge this. Thanks!

@james-mckenzie-cko
Copy link

@ianstormtaylor - I'm testing this with slate-react now and appears to be working. I've previously observed the error above with immer 8.0.1 and as @TheSpyder suggests, it no longer happens after #3850.

@rodrigonzalz
Copy link

If it is working can we get this merge?

@peterlundberg
Copy link

For our usage of slate, using resolutions to get 8.0.4 of immer with 0.62.0 of slate worked fine.

Pity there is no back-port from immer but perhaps this moves forward soon?

@o-alexandrov
Copy link

o-alexandrov commented May 4, 2021

@peterlundberg have you tried 9th version of immer? Just wondering whether that was stable for you

@TheSpyder
Copy link
Collaborator

I’ll merge this soon. I’ve been busy, sorry.

I may look at upgrading to a newer version of immer after it’s merged; feedback on whether they work would be great.

@peterlundberg
Copy link

Yes I did a quick test with 9.0.2 of immer in an explicit resolution with slate and slate-react 0.62.0. The way we use it it worked fine.

@ianstormtaylor
Copy link
Owner

@dependabot recreate

Bumps [immer](https://github.com/immerjs/immer) from 7.0.7 to 8.0.1.
- [Release notes](https://github.com/immerjs/immer/releases)
- [Commits](immerjs/immer@v7.0.7...v8.0.1)

Signed-off-by: dependabot[bot] <support@github.com>
@dependabot dependabot bot force-pushed the dependabot/npm_and_yarn/immer-8.0.1 branch from 15d0527 to b7c93bb Compare May 5, 2021 23:28
@ianstormtaylor ianstormtaylor merged commit 3b3c94a into main May 5, 2021
@dependabot dependabot bot deleted the dependabot/npm_and_yarn/immer-8.0.1 branch May 5, 2021 23:52
@lucas-stanford
Copy link

@ianstormtaylor is it possible to publish slate-history to npm? the latest version available doesn't have this change.

@hassenc
Copy link

hassenc commented Aug 4, 2021

@ianstormtaylor is it possible to publish slate-history to npm? the latest version available doesn't have this change.

Yes please, the current version of slate-history has a vulnerability (because of Immer < 8.0.1 )
https://snyk.io/vuln/SNYK-JS-IMMER-1019369

Can you publish this change?

@TheSpyder
Copy link
Collaborator

I'm not sure why slate-history even depends on immer. I'll remove it.

@TheSpyder
Copy link
Collaborator

#4430

@lucas-stanford
Copy link

lucas-stanford commented Aug 10, 2021

@TheSpyder I see this fix got published Thanks! https://www.npmjs.com/package/slate-history/v/0.65.3-202171015123, but it has a weird timestamp on it? What's the process to get a normal version published?

@TheSpyder
Copy link
Collaborator

@lucas-stanford we publish every build of main on the @next tag for bleeding edge testing. The release will happen when #4368 is merged - we’re planning to do that tomorrow.

dylans pushed a commit to dylans/slate that referenced this pull request Sep 13, 2021
Bumps [immer](https://github.com/immerjs/immer) from 7.0.7 to 8.0.1.
- [Release notes](https://github.com/immerjs/immer/releases)
- [Commits](immerjs/immer@v7.0.7...v8.0.1)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
aiwenar pushed a commit to openstax-poland/slate that referenced this pull request Jan 18, 2023
Bumps [immer](https://github.com/immerjs/immer) from 7.0.7 to 8.0.1.
- [Release notes](https://github.com/immerjs/immer/releases)
- [Commits](immerjs/immer@v7.0.7...v8.0.1)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.

None yet