arnelenero / simpler-state

The simplest app state management for React
https://simpler-state.js.org
MIT License
478 stars 16 forks source link

Immutable drafts with immer #12

Closed ottobonn closed 3 years ago

ottobonn commented 3 years ago

Describe the bug

Hi! I am trying to use simpler-state with immer and TypeScript in React Native and I am not able to mutate the document draft. Here's the relevant code:

interface State {
  readonly plants: [
    {
      readonly name?: string,
    }?
  ]
};

const initialState: State = {};

export const state = entity(initialState);

export function addPlant() {
  state.set(produce(draft => {
    draft.plants = draft.plants || [];
    draft.plants.push({});
  }));
}

This code results in the following error:

Unhandled promise rejection: TypeError: Attempted to assign to readonly property.]
at node_modules/immer/dist/immer.esm.js:1:15732 in <anonymous>
at node_modules/immer/dist/immer.esm.js:1:16123 in f.then$argument_1
at node_modules/immer/dist/immer.esm.js:1:15694 in <anonymous>
at node_modules/simpler-state/lib/entity.js:40:49 in createSetter
at node_modules/simpler-state/lib/persistence.js:26:16 in <anonymous>
at node_modules/react-native/Libraries/Pressability/Pressability.js:691:17 in _performTransitionSideEffects
at node_modules/react-native/Libraries/Pressability/Pressability.js:628:6 in _receiveSignal
at node_modules/react-native/Libraries/Pressability/Pressability.js:524:8 in responderEventHandlers.onResponderRelease
at [native code]:null in forEach
at [native code]:null in callFunctionReturnFlushedQueue

I am trying to initialize the plants array if it doesn't already exist.

To Reproduce

Run addPlant above.

Expected behavior

I would expect the whole draft document to be mutable.

arnelenero commented 3 years ago

Hi,

Since your plants is readonly in interface State, why don't you set the initialState to be { plants: [] } instead? This way, you wouldn't have to initialize it conditionally inside addPlant. This might solve the problem.

Also how come TS did not complain about the const initialState: State = {}; whereas plants is required in type State? Something seems to be wrong in the snippet above. Typo?

ottobonn commented 3 years ago

Thanks for the reply! I did try that first, but I would like to use this with a local file persistence so I can't guarantee that the file will be formatted correctly. It looks like immer is trying to produce a writable draft but for some reason the plants field stays read-only.

ottobonn commented 3 years ago

Hmm I am not able to repro in this REPL, so maybe it's something related to react-native. https://replit.com/@ottobonn/SimplerStateTS#index.ts

arnelenero commented 3 years ago

I think using immer is enough to ensure immutable state updates, and so you can probably remove the readonly constraint from your interface. I feel like this is causing the issue, and conceptually it seems redundant.

arnelenero commented 3 years ago

@ottobonn Hi. Do you still suspect something is wrong with SimpleR State itself? If not, we can close the issue. Thanks.

ottobonn commented 3 years ago

Hi again! I just figured it out. I was using the persistence option with SimpleR State to save the JSON to the filesystem, but I didn't notice that it handles stringification for me. As a result, I was double-serializing the JSON saved to the filesystem, and when I loaded it I wasn't deserializing it, so it was a string instead of an object. When I tried to mutate it with immer, I was actually trying to mutate a string, so it was throwing an error.

Thanks for the reply and for the great library!

ottobonn commented 3 years ago

For reference, it was something like this:

(https://replit.com/@ottobonn/SimplerStateTS)

const {entity, persistence} = require('simpler-state');
const {produce} = require('immer');

let fakeFile = '"\\"{}\\""';

const jsonStorage = {
  getItem(key: string) {
    return fakeFile;
  },
  setItem(key: string, value: any) {
    fakeFile = JSON.stringify(value);
  },
};

const state = entity({}, [
  persistence('test', {storage: jsonStorage}),
]);

function addToArray() {
  state.set(produce((draft: any) => {
    draft.array = draft.array || [];
    draft.array.push({});
  }));
}

console.log(state.get())

addToArray();
addToArray();

console.log(state.get())

I ended up with double-serialized JSON in the file, so when I parsed it, it was still a string. In the REPL, the error is pretty clear:

"{}"
TypeError: Cannot create property 'array' on string '"{}"'
    at state.set.produce (index.ts:18:21)
    at recipe (/home/runner/SimplerStateTS/node_modules/immer/src/core/immerClass.ts:78:51)
    at Immer.produce (/home/runner/SimplerStateTS/node_modules/immer/src/core/immerClass.ts:116:13)
    at curriedProduce (/home/runner/SimplerStateTS/node_modules/immer/src/core/immerClass.ts:78:17)
    at /home/runner/SimplerStateTS/node_modules/simpler-state/lib/entity.js:46:61
    at Object.set (/home/runner/SimplerStateTS/node_modules/simpler-state/lib/persistence.js:32:17)
    at addToArray (index.ts:17:11)
    at index.ts:23:1
    at Script.runInThisContext (vm.js:122:20)
    at startRepl (/usr/local/lib/node_modules/ts-node-fm/src/bin.ts:157:12)

However, in my react-native project, the error just said "Attemped to assign read-only property," so it wasn't so obvious. I would still like to understand exactly why that was the error, rather than the one in the REPL.

arnelenero commented 3 years ago

Thanks for posting the details. This could help others who, in the future, might also run into a similar scenario.