Using immer from libraries can lead to silent bugs in downstream projects

[bard]

• Feature request: ______
  • I am willing to implement this myself
• Issue: Using immer in libraries can lead to silent bugs in downstream projects
  • Version: 2.1.0
  • SIMPLE Reproduction: https://github.com/bard/repro-immer-in-libraries
  • Expected behavior: depending on immer in libraries would be transparent to downstream projects
  • Observed behavior: behavior in downstream project changes in complex ways if both it and library are using immer
  • Occurs when using Proxies (use setUseProxies(true))
  • Occurs in the ES5 implementation (use setUseProxies(false))

Context: I have a Redux subreducer that uses immer and I'm making it available to other Redux projects as a packaged library. It's up to downstream whether to also use immer or not. This sort of works, with some precautions, but I suspect I'll hit serious trouble later.

A contrived example of a downstream project that uses both immer and the library:

import produce from 'immer'
import capitalizeNameSubreducer from 'capitalize-name-subreducer'

const appReducer = (state, action) => produce(state, (draft) => {
  switch (action.type) {
    case 'CAPITALIZE_NAME':
      capitalizeNameSubreducer(draft)
      break
  }
})

Downstream project that uses the library but doesn't use immer:

import capitalizeNameSubreducer from 'capitalize-name-subreducer'

const appReducer = (state, action) => {
  switch (action.type) {
    case 'CAPITALIZE_NAME':
      return capitalizeNameSubreducer(state)
      break
    default:
      return state
  }
}

(Downstream projects know that capitalizeNameSubreducer expects a certain state shape and are fine with that.)

The subreducer itself:

import produce, { isDraft } from 'immer'

// applies immer if upstream isn't using it, otherwise re-uses already proxied object
const withImmer = (state, worker) => isDraft(state)
  ? worker(state, worker)
  : produce(state, worker)

const capitalizeNameSubreducer = (state) => withImmer(state, (draft) => {
  draft.name = draft.name.replace(/^\w/, c => c.toUpperCase())
})

The problem is in how to deal with the dependency on immer within the capitalize-name-subreducer package.

The short story is that any scenario that leads to two copies of immer (project/node_modules/immer and project/node_modules/library/node_modules/immer) won't work, because (if I get it right) immer uses symbols to mark objects as drafts and they are not === across modules.

If immer is among the library's peerDependencies...

It works but it's one more thing for downstream authors to be aware of and fairly arbitrary if they're not using immer already.

If immer is among the library's dependencies...

It works if:

• downstream project is not using immer, or
• downstream project is using a version of immer that can be de-duplicated from the one in capitalize-name-subreducer

It does not work if:

1. downstream project uses a version of immer that cannot be de-duplicated
2. downstream project is on my own disk and I'm importing my library via yarn link or npm link (which always leads to two installations of immer being present, even when they can in principle de-duplicated)

Case 2 is an annoyance but case 1 is a potentially life endangering threat. (Ok, slightly exaggerating here.) Imagine downstream project and library using immer@2. Two months later library uses immer@3. Project does an upgrade and brings in immer@3 as transitive dependency, while immer@2 still is present. Stuff stops working with no clear indication as to why.

If immer is among the library's devDependencies...

This works if:

• downstream project is already using immer

This does not work if:

• downstream project is not using immer

The reason it does not work is, of course, that installing the library doesn't bring its devDependencies into the downstream project, so if immer isn't already there, it will be missing.

---

Reproduction for (working) scenario "immer not in library's node_modules, immer in project's node_modules":

$ git clone https://github.com/bard/repro-immer-in-libraries
$ cd repro-immer-in-libraries/downstream-project
$ yarn install
$ node test.js
{"name":"john"} -> {"name":"John"}

Reproduction for (non-working) scenario "immer in library's node_modules, immer in project's_modules" (note: this leverages the bug^Wfact that adding a local dependency with yarn doesn't dedupe modules no-matter-what, to simulate the scenario):

$ git clone https://github.com/bard/repro-immer-in-libraries
$ cd repro-immer-in-libraries/capitalize-name-subreducer && yarn install
$ cd ../downstream-project && yarn install
$ node test.js
{"name":"john"} -> {"name":"john"}

---

Apologies for the lengthy description but it was either giving up length or accuracy.

Is there some strategy to address the issue?

[aleclarson]

Thanks for reporting this! 👍

You want capitalizeNameSubreducer to behave differently based on whether the object passed to it is a draft. When a draft is passed, mutate it directly. Otherwise, produce a copy.

The problem (as you've identified) is that isDraft always returns false for drafts created by a separate installation of immer. This is unintended behavior, and I consider it a bug.

To avoid this, we can use the global scope to share Immer's internal symbols between all installations of immer. Thoughts on this, @mweststrate?

For DRAFTABLE and DRAFT_STATE, we can use Symbol.for instead of the global scope. But NOTHING would need the global scope, if we think it's necessary for NOTHING to be recognized across different installations.

[mweststrate]

I Agree :)

…

On Tue, Mar 5, 2019 at 2:36 PM Alec Larson ***@***.***> wrote: Thanks for reporting this! 👍 You want capitalizeNameSubreducer to behave differently based on whether the object passed to it is a draft. When a draft is passed, mutate it directly. Otherwise, produce a copy. The problem (as you've identified) is that isDraft always returns false for drafts created by a separate installation of immer. This is unintended behavior, and I consider it a bug. To avoid this, we can use the global scope to share Immer's internal symbols between all installations of immer. Thoughts on this, @mweststrate <https://github.com/mweststrate>? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#322 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABvGhMn6DcyLZtfA3OozdQZ7D_hzT-gAks5vTnL2gaJpZM4beTDG> .

[aleclarson]

🎉 This issue has been resolved in version 2.1.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[bard]

Fantastic, thank you!