TypeScript, store, produce, draft, and readonly

[astoilkov]

Proposal

Remove TypeScript readonly from the draft type of the produce function. This is the way immer works — https://github.com/immerjs/immer/blob/8e12c78073c5f7c3a556a76dd780125b23bb12a3/src/types/types-external.ts#L35-L48.

Code example

The code below produces a TypeScript error and it shouldn't. The code will work correctly during runtime.

type AppState = {
  readonly todos: readonly {
    text: string
    done: boolean
  }[]
}

const [_store, setStore] = createStore<AppState>({
  todos: []
})

setStore(produce(draft => {
  // ⚠️ TypeScript error here:
  // "Property 'push' does not exist on type 'readonly { text: string; done: boolean; }[]'.(2339)"
  draft.todos.push({
    text: 'new todo',
    done: false
  })
}))

Current implementation

const [store] = createStore({ todos: [] })
// ⚠️ Code below prints a warning in browser console
store.todos.push({})

Currently, Solid will warn when you try to update the store directly. This is great. However, if I add readonly to the type I will start getting compile-time errors instead of runtime errors and this improves DX.

---

P.S.: I searched for similar proposals but couldn't find one. I'm new to solid, so I'm sorry if this has been discussed elsewhere.

[JorrenH]

Solid stores have read/write segregation, which means when creating the following store

const [store, setStore] = createStore({ todos: [] ]);

The store variable is a read-only accesor to the store, while setStore is a store writer. I think the meaning of a readonly store prop should be that it can never mutate, so the first behavior seems correct.

I do agree store.todos.push({}) should result in a type-error, as store is (conceptually) a read-only variable. The return type for store could be made more strict to enforce this, but there might be reasons why this is not yet the case.

[astoilkov]

Aha, yeah, your idea might be better as the user won't need to add readonly to everything. Nice!

[otonashixav]

Trying to add or remove the readonly modifier recursively is actually quite troublesome. Previously the store returned by createStore was completely readonly, but self-referencing types and generic types don't play well with recursive readonly, producing confusing error messages. Also many people were confused as to why their functions accepting non-readonly-typed store types wouldn't accept their stores.

Typescript also has no way to differentiate classes and plain objects, so classes would be wrongly made (not) readonly unless manually configured otherwise by extending an interface.

Maybe most importantly, another way of indicating that something is actually runtime-readonly (e.g. a property getter) would be necessary, otherwise we would be introducing the possibility of a runtime error in exchange for moving a runtime warning to a compile time error.

As such I don't see this proposal working. You can still manually type the store and not the setter as readonly, or overload produce to make draft mutable with DeepMutable (though I believe this causes inference problems which require some TS hackery to get around).

[astoilkov]

I've been thinking about this since I posted the proposal here and I agree. I can manually type everything if I want to. I believe it has benefits for onboarding users but has some drawbacks for long-term users. I see the benefits for long users as well and I believe you are talking about edge cases but this is a core part of Solid and I wouldn't touch it unless 100% sure. Conclusion — I agree with you that it's not a good idea.