Skip to content

Commit

Permalink
chore: fix some build issues (#701)
Browse files Browse the repository at this point in the history
* clearer error when plugin is missing

* chore: fix travis build not failing, fixes #688 (?)

* fix: make plugin loading idempotent, fixes #692

* docs: Organize performance and pitfalls, and document nested produce behavior. Fixes #694

* chore: fix prod-build-issues
  • Loading branch information
mweststrate committed Nov 17, 2020
1 parent 678e541 commit 31684f2
Show file tree
Hide file tree
Showing 15 changed files with 224 additions and 145 deletions.
11 changes: 6 additions & 5 deletions .travis.yml
@@ -1,7 +1,7 @@
language: node_js
node_js:
- "10.18.1"
# - "node"
# - "10.18.1"
- "node"
env:
- NODE_ENV=TEST
cache:
Expand All @@ -12,12 +12,13 @@ before_script:
- yarn global add if-node-version
script:
- yarn build || travis_terminate 1
- if-node-version 10 || { yarn test && travis_terminate 0; }
- yarn coveralls
- yarn test || travis_terminate 1
- yarn coveralls || travis_terminate 0
jobs:
include:
- stage: deploy
if: branch == master && !fork
script:
- NODE_ENV= yarn build
- yarn test || travis_terminate 1
- NODE_ENV= yarn build || travis_terminate 1
- yarn semantic-release
80 changes: 32 additions & 48 deletions __tests__/__prod_snapshots__/base.js.snap

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions __tests__/__prod_snapshots__/patch.js.snap
@@ -1,7 +1,7 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`applyPatches throws when \`op\` is not "add", "replace", nor "remove" 1`] = `"[Immer] minified error nr: 17 copy. Find the full error at: https://bit.ly/3cXEKWf"`;
exports[`applyPatches throws when \`op\` is not "add", "replace", nor "remove" 1`] = `"[Immer] minified error nr: 17 'copy'. Find the full error at: https://bit.ly/3cXEKWf"`;

exports[`applyPatches throws when \`path\` cannot be resolved 1`] = `"[Immer] minified error nr: 15 a/b. Find the full error at: https://bit.ly/3cXEKWf"`;
exports[`applyPatches throws when \`path\` cannot be resolved 1`] = `"[Immer] minified error nr: 15 'a/b'. Find the full error at: https://bit.ly/3cXEKWf"`;

exports[`applyPatches throws when \`path\` cannot be resolved 2`] = `"[Immer] minified error nr: 15 a/b/c. Find the full error at: https://bit.ly/3cXEKWf"`;
exports[`applyPatches throws when \`path\` cannot be resolved 2`] = `"[Immer] minified error nr: 15 'a/b/c'. Find the full error at: https://bit.ly/3cXEKWf"`;
10 changes: 5 additions & 5 deletions __tests__/__prod_snapshots__/plugins.js.snap
@@ -1,11 +1,11 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`ES5 plugins should throw if no proxies are available error when using ES5 1`] = `"[Immer] minified error nr: 19 ES5. Find the full error at: https://bit.ly/3cXEKWf"`;
exports[`ES5 plugins should throw if no proxies are available error when using ES5 1`] = `"[Immer] minified error nr: 18 'ES5'. Find the full error at: https://bit.ly/3cXEKWf"`;

exports[`error when using Maps 1`] = `"[Immer] minified error nr: 19 MapSet. Find the full error at: https://bit.ly/3cXEKWf"`;
exports[`error when using Maps 1`] = `"[Immer] minified error nr: 18 'MapSet'. Find the full error at: https://bit.ly/3cXEKWf"`;

exports[`error when using patches - 1 1`] = `"[Immer] minified error nr: 19 Patches. Find the full error at: https://bit.ly/3cXEKWf"`;
exports[`error when using patches - 1 1`] = `"[Immer] minified error nr: 18 'Patches'. Find the full error at: https://bit.ly/3cXEKWf"`;

exports[`error when using patches - 2 1`] = `"[Immer] minified error nr: 19 Patches. Find the full error at: https://bit.ly/3cXEKWf"`;
exports[`error when using patches - 2 1`] = `"[Immer] minified error nr: 18 'Patches'. Find the full error at: https://bit.ly/3cXEKWf"`;

exports[`error when using patches - 3 1`] = `"[Immer] minified error nr: 19 Patches. Find the full error at: https://bit.ly/3cXEKWf"`;
exports[`error when using patches - 3 1`] = `"[Immer] minified error nr: 18 'Patches'. Find the full error at: https://bit.ly/3cXEKWf"`;
107 changes: 56 additions & 51 deletions __tests__/base.js
Expand Up @@ -16,6 +16,8 @@ jest.setTimeout(1000)

enableAllPlugins()

const isProd = process.env.NODE_ENV === "production"

test("immer should have no dependencies", () => {
expect(require("../package.json").dependencies).toBeUndefined()
})
Expand Down Expand Up @@ -1145,65 +1147,66 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) {
})

// NOTE: ES5 drafts only protect existing properties when revoked.
it("revokes the draft once produce returns", () => {
const expectRevoked = (fn, shouldThrow = true) => {
if (shouldThrow) expect(fn).toThrowErrorMatchingSnapshot()
else expect(fn).not.toThrow()
}
if (!isProd)
it("revokes the draft once produce returns", () => {
const expectRevoked = (fn, shouldThrow = true) => {
if (shouldThrow) expect(fn).toThrowErrorMatchingSnapshot()
else expect(fn).not.toThrow()
}

// Test object drafts:
let draft
produce({a: 1, b: 1}, s => {
draft = s
delete s.b
})
// Test object drafts:
let draft
produce({a: 1, b: 1}, s => {
draft = s
delete s.b
})

// Access known property on object draft.
expectRevoked(() => {
draft.a
})
// Access known property on object draft.
expectRevoked(() => {
draft.a
})

// Assign known property on object draft.
expectRevoked(() => {
draft.a = true
})
// Assign known property on object draft.
expectRevoked(() => {
draft.a = true
})

// Access unknown property on object draft.
expectRevoked(() => {
draft.z
}, useProxies)
// Access unknown property on object draft.
expectRevoked(() => {
draft.z
}, useProxies)

// Assign unknown property on object draft.
expectRevoked(() => {
draft.z = true
}, useProxies)
// Assign unknown property on object draft.
expectRevoked(() => {
draft.z = true
}, useProxies)

// Test array drafts:
produce([1, 2], s => {
draft = s
s.pop()
})
// Test array drafts:
produce([1, 2], s => {
draft = s
s.pop()
})

// Access known index of an array draft.
expectRevoked(() => {
draft[0]
})
// Access known index of an array draft.
expectRevoked(() => {
draft[0]
})

// Assign known index of an array draft.
expectRevoked(() => {
draft[0] = true
})
// Assign known index of an array draft.
expectRevoked(() => {
draft[0] = true
})

// Access unknown index of an array draft.
expectRevoked(() => {
draft[1]
}, useProxies)
// Access unknown index of an array draft.
expectRevoked(() => {
draft[1]
}, useProxies)

// Assign unknown index of an array draft.
expectRevoked(() => {
draft[1] = true
}, useProxies)
})
// Assign unknown index of an array draft.
expectRevoked(() => {
draft[1] = true
}, useProxies)
})

it("can access a child draft that was created before the draft was modified", () => {
produce({a: {}}, s => {
Expand Down Expand Up @@ -1697,7 +1700,7 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) {
},
e => {
expect(e).toBe(err)
expect(() => draft.a).toThrowErrorMatchingSnapshot()
if (!isProd) expect(() => draft.a).toThrowErrorMatchingSnapshot()
}
)
})
Expand Down Expand Up @@ -2418,7 +2421,9 @@ function testLiteralTypes(produce) {
draft.foo = true
})
).toThrowError(
"produce can only be called on things that are draftable"
isProd
? "[Immer] minified error nr: 21"
: "produce can only be called on things that are draftable"
)
})
} else {
Expand Down
8 changes: 7 additions & 1 deletion __tests__/current.js
Expand Up @@ -14,6 +14,8 @@ enableAllPlugins()
runTests("proxy", true)
runTests("es5", false)

const isProd = process.env.NODE_ENV === "production"

function runTests(name, useProxies) {
describe("current - " + name, () => {
beforeAll(() => {
Expand All @@ -24,7 +26,11 @@ function runTests(name, useProxies) {
it("must be called on draft", () => {
expect(() => {
current({})
}).toThrowError("[Immer] 'current' expects a draft, got: [object Object]")
}).toThrowError(
isProd
? "[Immer] minified error nr: 22 '[object Object]'. Find the full error at: https://bit.ly/3cXEKWf"
: "[Immer] 'current' expects a draft, got: [object Object]"
)
})

it("can handle simple arrays", () => {
Expand Down
21 changes: 12 additions & 9 deletions __tests__/manual.js
Expand Up @@ -10,6 +10,8 @@ import {

enableAllPlugins()

const isProd = process.env.NODE_ENV === "production"

runTests("proxy", true)
runTests("es5", false)

Expand Down Expand Up @@ -41,16 +43,17 @@ function runTests(name, useProxies) {
expect(state).toEqual([{}, {}, {}])
})

it("cannot modify after finish", () => {
const state = {a: 1}
if (!isProd)
it("cannot modify after finish", () => {
const state = {a: 1}

const draft = createDraft(state)
draft.a = 2
expect(finishDraft(draft)).toEqual({a: 2})
expect(() => {
draft.a = 3
}).toThrowErrorMatchingSnapshot()
})
const draft = createDraft(state)
draft.a = 2
expect(finishDraft(draft)).toEqual({a: 2})
expect(() => {
draft.a = 3
}).toThrowErrorMatchingSnapshot()
})

it("should support patches drafts", () => {
const state = {a: 1}
Expand Down
17 changes: 16 additions & 1 deletion __tests__/map-set.js
Expand Up @@ -5,7 +5,8 @@ import {
original,
isDraft,
immerable,
enableAllPlugins
enableAllPlugins,
enableMapSet
} from "../src/immer"
import {each, shallowCopy, isEnumerable, DRAFT_STATE} from "../src/common"

Expand Down Expand Up @@ -280,5 +281,19 @@ function runBaseTest(name, useProxies, autoFreeze, useListener) {
})
expect(result).toBe(set)
})

test("#692 - idempotent plugin loading", () => {
let mapType1
produce(new Map(), draft => {
mapType1 = draft.constructor
})

enableMapSet()
let mapType2
produce(new Map(), draft => {
mapType2 = draft.constructor
})
expect(mapType1).toBe(mapType2)
})
})
}
18 changes: 14 additions & 4 deletions __tests__/original.js
Expand Up @@ -3,6 +3,8 @@ import produce, {original, setUseProxies, enableAllPlugins} from "../src/immer"

enableAllPlugins()

const isProd = process.env.NODE_ENV === "production"

describe("original", () => {
const baseState = {
a: [],
Expand Down Expand Up @@ -40,20 +42,28 @@ describe("original", () => {
draftState.c = {}
draftState.d = 3
expect(() => original(draftState.c)).toThrowErrorMatchingInlineSnapshot(
`"[Immer] 'original' expects a draft, got: [object Object]"`
isProd
? `"[Immer] minified error nr: 23 '[object Object]'. Find the full error at: https://bit.ly/3cXEKWf"`
: `"[Immer] 'original' expects a draft, got: [object Object]"`
)
expect(() => original(draftState.d)).toThrowErrorMatchingInlineSnapshot(
`"[Immer] 'original' expects a draft, got: 3"`
isProd
? `"[Immer] minified error nr: 23 '3'. Find the full error at: https://bit.ly/3cXEKWf"`
: `"[Immer] 'original' expects a draft, got: 3"`
)
})
})

it("should return undefined for an object that is not proxied", () => {
expect(() => original({})).toThrowErrorMatchingInlineSnapshot(
`"[Immer] 'original' expects a draft, got: [object Object]"`
isProd
? `"[Immer] minified error nr: 23 '[object Object]'. Find the full error at: https://bit.ly/3cXEKWf"`
: `"[Immer] 'original' expects a draft, got: [object Object]"`
)
expect(() => original(3)).toThrowErrorMatchingInlineSnapshot(
`"[Immer] 'original' expects a draft, got: 3"`
isProd
? `"[Immer] minified error nr: 23 '3'. Find the full error at: https://bit.ly/3cXEKWf"`
: `"[Immer] 'original' expects a draft, got: 3"`
)
})
})
18 changes: 15 additions & 3 deletions docs/performance.md
Expand Up @@ -42,6 +42,18 @@ Most important observation:

## Performance tips

- When adding a large data set to the state tree in an Immer producer (for example data received from a JSON endpoint), it is worth to call `Object.freeze(json)` on the root of the data to be added first. This will allow Immer to add the new data to the tree faster, as it will skip freezing it, or searching the tree for any changes (drafts) that might be made.
- Immer will convert anything you read in a draft recursively into a draft as well. If you have expensive side effect free operations on a draft that involves a lot of reading, for example finding an index using `find(Index)` in a very large array, you can speed this up by first doing the search, and only call the `produce` function once you know the index. Thereby preventing Immer to turn everything that was searched for in a draft. Or, perform the search on the original value of a draft, by using `original(someDraft)`, which boils to the same thing.
- Always try to pull produce 'up', for example `for (let x of y) produce(base, d => d.push(x))` is exponentially slower than `produce(base, d => { for (let x of y) d.push(x)})`
### Pre-freeze data

When adding a large data set to the state tree in an Immer producer (for example data received from a JSON endpoint), it is worth to call `Object.freeze(json)` on the root of the data to be added first. This will allow Immer to add the new data to the tree faster, as it will skip freezing it, or searching the tree for any changes (drafts) that might be made.

### You can always opt-out

Realize that immer is opt-in everywhere, so it is perfectly fine to manually write super performance critical reducers, and use immer for all the normal ones. Even from within a producer you opt-out from Immer for certain parts of your logic by using utilies `original` or `current` and perform some of your operations on plain JavaScript objects.

### For expensive search operations, read from the original state, not the draft

Immer will convert anything you read in a draft recursively into a draft as well. If you have expensive side effect free operations on a draft that involves a lot of reading, for example finding an index using `find(Index)` in a very large array, you can speed this up by first doing the search, and only call the `produce` function once you know the index. Thereby preventing Immer to turn everything that was searched for in a draft. Or, alternatively, perform the search on the original value of a draft, by using `original(someDraft)`, which boils to the same thing.

### Pull produce as far up as possible

Always try to pull produce 'up', for example `for (let x of y) produce(base, d => d.push(x))` is exponentially slower than `produce(base, d => { for (let x of y) d.push(x)})`

0 comments on commit 31684f2

Please sign in to comment.