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

Added support for frozen trees, and automatically freeze generated trees #15

Merged
merged 6 commits into from
Jan 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 3 additions & 38 deletions __tests__/base.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
"use strict"
import immer from ".."

describe("base", () => {
Expand Down Expand Up @@ -137,7 +138,6 @@ describe("base", () => {

it("reusing object should work", () => {
const nextState = immer(baseState, s => {
debugger
const obj = s.anObject
delete s.anObject
s.messy = obj
Expand Down Expand Up @@ -204,14 +204,13 @@ describe("base", () => {
}).toThrowError(/^Cannot perform.*on a proxy that has been revoked/)
expect(() => {
const aProp = proxy.aProp
}).toThrowError(/^Cannot perform.*on a proxy that has been revoked/)
}).toThrowError(/^Cannot perform.*on a proxy that has been revoked/)

expect(nextState).not.toBe(baseState)
expect(baseState.aProp).toBe("hi")
expect(nextState.aProp).toBe("hello")

})

afterEach(() => {
expect(baseState).toBe(origBaseState)
expect(baseState).toEqual(createBaseState())
Expand All @@ -230,37 +229,3 @@ describe("base", () => {
}
}
})

describe("readme example", () => {
it("works", () => {
const baseState = [
{
todo: "Learn typescript",
done: true,
},
{
todo: "Try immer",
done: false,
},
]

const nextState = immer(baseState, state => {
state.push({todo: "Tweet about it"})
state[1].done = true
})

// the new item is only added to the next state,
// base state is unmodified
expect(baseState.length).toBe(2)
expect(nextState.length).toBe(3)

// same for the changed 'done' prop
expect(baseState[1].done).toBe(false)
expect(nextState[1].done).toBe(true)

// unchanged data is structurally shared
expect(nextState[0]).toBe(baseState[0])
// changed data not (dûh)
expect(nextState[1]).not.toBe(baseState[1])
})
})
49 changes: 49 additions & 0 deletions __tests__/frozen.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
"use strict"
import immer from ".."

describe("auto freeze", () => {
const baseState = {
object: {a: 1},
array: [1, 2],
}

it("should freeze objects after modifications", () => {
expect(Object.isFrozen(baseState.object)).toBe(false) // initially not frozen
const next = immer(baseState, draft => {
draft.object.c = 2
})
expect(Object.isFrozen(next.object)).toBe(true)
expect(Object.isFrozen(next)).toBe(true)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also safely assert that next.array is not forzen in the process ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

expect(Object.isFrozen(next.array)).toBe(false)

expect(() => {
next.object.a = 2
}).toThrow(/Cannot assign to read only property/)
})

it("should freeze arrays after modifications", () => {
expect(Object.isFrozen(baseState.object)).toBe(false) // initially not frozen
const next = immer(baseState, draft => {
draft.array.push(3)
})
expect(Object.isFrozen(next.object)).toBe(false) // not touched
expect(Object.isFrozen(next)).toBe(true)
expect(Object.isFrozen(next.array)).toBe(true)

expect(() => {
next.array.shift()
}).toThrow(/Cannot add\/remove sealed array elements/)
})

it("can handle already frozen trees", () => {
const a = []
const b = {a: a}
Object.freeze(a)
Object.freeze(b)
const n = immer(b, draft => {
draft.c = true
draft.a.push(3)
})
expect(n).toEqual({c: true, a: [3]})
})
})
36 changes: 36 additions & 0 deletions __tests__/readme.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
"use strict"
import immer from ".."

describe("readme example", () => {
it("works", () => {
const baseState = [
{
todo: "Learn typescript",
done: true,
},
{
todo: "Try immer",
done: false,
},
]

const nextState = immer(baseState, draft => {
draft.push({todo: "Tweet about it"})
draft[1].done = true
})

// the new item is only added to the next state,
// base state is unmodified
expect(baseState.length).toBe(2)
expect(nextState.length).toBe(3)

// same for the changed 'done' prop
expect(baseState[1].done).toBe(false)
expect(nextState[1].done).toBe(true)

// unchanged data is structurally shared
expect(nextState[0]).toBe(baseState[0])
// changed data not (dûh)
expect(nextState[1]).not.toBe(baseState[1])
})
})
92 changes: 72 additions & 20 deletions immer.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
"use strict"
// @ts-check

/**
* @typedef {Object} RevocableProxy
* @property {any} proxy
* @property {Function} revoke
*/

// @ts-check
const IMMER_PROXY = Symbol("immer-proxy") // TODO: create per closure, to avoid sharing proxies between multiple immer version

const isProxySymbol = Symbol("immer-proxy")
// This property indicates that the current object is cloned for another object,
// to make sure the proxy of a frozen object is writeable
const CLONE_TARGET = Symbol("immer-clone-target")

let autoFreeze = true

/**
* Immer takes a state, and runs a function against it.
Expand All @@ -19,7 +26,6 @@ const isProxySymbol = Symbol("immer-proxy")
* @returns {any} a new state, or the base state if nothing was modified
*/
function immer(baseState, thunk) {

/**
* Maps baseState objects to revocable proxies
* @type {Map<Object,RevocableProxy>}
Expand All @@ -31,7 +37,7 @@ function immer(baseState, thunk) {

const objectTraps = {
get(target, prop) {
if (prop === isProxySymbol) return target
if (prop === IMMER_PROXY) return target
return createProxy(getCurrentSource(target)[prop])
},
has(target, prop) {
Expand All @@ -46,7 +52,7 @@ function immer(baseState, thunk) {
if (current !== newValue) {
const copy = getOrCreateCopy(target)
copy[prop] = isProxy(newValue)
? newValue[isProxySymbol]
? newValue[IMMER_PROXY]
: newValue
}
return true
Expand All @@ -61,10 +67,16 @@ function immer(baseState, thunk) {
// creates a copy for a base object if there ain't one
function getOrCreateCopy(base) {
let copy = copies.get(base)
if (!copy) {
copy = Array.isArray(base) ? base.slice() : Object.assign({}, base)
copies.set(base, copy)
if (copy) return copy
const cloneTarget = base[CLONE_TARGET]
if (cloneTarget) {
// base is a clone already (source was frozen), no need to create addtional copy
copies.set(cloneTarget, base)
return base
}
// create a fresh copy
copy = Array.isArray(base) ? base.slice() : Object.assign({}, base)
copies.set(base, copy)
return copy
}

Expand All @@ -73,14 +85,29 @@ function immer(baseState, thunk) {
const copy = copies.get(base)
return copy || base
}

// creates a proxy for plain objects / arrays
function createProxy(base) {
if (isPlainObject(base) || Array.isArray(base)) {
if (isProxy(base)) return base // avoid double wrapping
if (revocableProxies.has(base)) return revocableProxies.get(base).proxy
//const proxy = new Proxy(base, objectTraps)
const revocableProxy = Proxy.revocable(base, objectTraps)
if (revocableProxies.has(base))
return revocableProxies.get(base).proxy
let proxyTarget
// special case, if the base tree is frozen, we cannot modify it's proxy it in strict mode, clone first.
if (Object.isFrozen(base)) {
proxyTarget = Array.isArray(base)
? base.slice()
: Object.assign({}, base)
Object.defineProperty(proxyTarget, CLONE_TARGET, {
enumerable: false,
value: base,
configurable: true,
})
} else {
proxyTarget = base
}
// create the proxy
const revocableProxy = Proxy.revocable(proxyTarget, objectTraps)
revocableProxies.set(base, revocableProxy)
return revocableProxy.proxy
}
Expand Down Expand Up @@ -115,20 +142,22 @@ function immer(baseState, thunk) {

function finalizeObject(thing) {
if (!hasChanges(thing)) return thing
const copy = getOrCreateCopy(thing)
const copy = getOrCreateCopy(thing) // TODO: getOrCreate is weird here..
Object.keys(copy).forEach(prop => {
copy[prop] = finalize(copy[prop])
})
return copy
delete copy[CLONE_TARGET]
return freeze(copy)
}

function finalizeArray(thing) {
if (!hasChanges(thing)) return thing
const copy = getOrCreateCopy(thing)
const copy = getOrCreateCopy(thing) // TODO: getOrCreate is weird here..
copy.forEach((value, index) => {
copy[index] = finalize(copy[index])
})
return copy
delete copy[CLONE_TARGET]
return freeze(copy)
}

// create proxy for root
Expand All @@ -148,10 +177,10 @@ function immer(baseState, thunk) {

/**
* Revoke all the proxies stored in the revocableProxies map
*
*
* @param {Map<Object,RevocableProxy>} revocableProxies
*/
function revoke (revocableProxies){
function revoke(revocableProxies) {
for (var revocableProxy of revocableProxies.values()) {
revocableProxy.revoke()
}
Expand All @@ -164,7 +193,30 @@ function isPlainObject(value) {
}

function isProxy(value) {
return !!value && !!value[isProxySymbol]
return !!value && !!value[IMMER_PROXY]
}

function freeze(value) {
if (autoFreeze) {
Object.freeze(value)
}
return value
}

/**
* Automatically freezes any state trees generated by immer.
* This protects against accidental modifications of the state tree outside of an immer function.
* This comes with a performance impact, so it is recommended to disable this option in production.
* It is by default enabled.
*
* @returns {void}
*/
function setAutoFreeze(enableAutoFreeze) {
autoFreeze = enableAutoFreeze
}

module.exports = immer
Object.defineProperty(exports, "__esModule", {
value: true,
})
module.exports.default = immer
module.exports.setAutoFreeze = setAutoFreeze
15 changes: 13 additions & 2 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,20 @@
* Immer takes a state, and runs a function against it.
* That function can freely mutate the state, as it will create copies-on-write.
* This means that the original state will stay unchanged, and once the function finishes, the modified state is returned
*
*
* @param currentState - the state to start with
* @param thunk - function that receives a proxy of the current state as first argument and which can be freely modified
* @returns The next state: a new state, or the current state if nothing was modified
*/
export function immer<S = any>(currentState: S, thunk: (draftState: S) => void): S;
export default function<S = any>(
currentState: S,
thunk: (draftState: S) => void,
): S

/**
* Automatically freezes any state trees generated by immer.
* This protects against accidental modifications of the state tree outside of an immer function.
* This comes with a performance impact, so it is recommended to disable this option in production.
* It is by default enabled.
*/
export function setAutoFreeze(autoFreeze: boolean)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return type should be void instead of implicit any

export function setAutoFreeze(autoFreeze: boolean):void

This is to prevent the use of setAutoFreeze return value in an unchecked way. Here is an example:

const a = setAutoFreeze(false)
a.test(); // error if the return type of setAutoFreeze is void, but will be ok if return type is any

15 changes: 15 additions & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,15 @@ expect(nextState[1]).not.toBe(baseState[1])
* Small, dependency free library with minimal api surface
* No accidental mutations of current state, but intentional mutations of a draft state

## Auto freezing

Immer automatically freezes any state trees that are modified using `immer.
This protects against accidental modifications of the state tree outside of an immer function.
This comes with a performance impact, so it is recommended to disable this option in production.
It is by default enabled.

Use `setAutoFreeze(true / false)` to turn this feature on or off.

## Reducer Example

A lot of words; here is a simple example of the difference that this approach could make in practice.
Expand Down Expand Up @@ -154,6 +163,12 @@ Creating middleware or a reducer wrapper that applies `immer` automatically is l

## Changelog

### 0.0.5

* Fixed `immer` function export, it is now properly exposed as `default`
* Immer now automatically freezes any state modifications made. Turn this is using `setAutoFreeze(false)`
* Added support for frozen state trees in strict mode.

### 0.0.4 (31-12-2017)

* Added typescript typings [#11](https://github.com/mweststrate/immer/pull/11) by [@benbraou](https://github.com/benbraou)
Expand Down