fix!: Prevent calling set on the return value of 'computed()'

BREAKING CHANGE: 'set()' method now only exists if a 'set()' is provided

Signed-off-by: Sebastian Malton <[email protected]>

.changeset/fluffy-shoes-return.md

@@ -0,0 +1,13 @@
+---
+"mobx": major
+---
+
+Only expose IComputedValue.set as a typescript type when it won't throw an error
+
+The breaking change is that it is now a typescript compile error to call the `set(value: T)` method on an `IComputedValue` that is not writable.
+This is a breaking change because it is possible to create an `IComputedValue` that is not writable, and then call `set` on it.
+This will now throw a runtime error.
+
+This change was made to help with refactoring code, especially between `IObservableValue<T>` and `IComputedValue<T>` which both have a similar interface but one of them throws more errors.
+
+To fix your code, you will need to change the type annotations for any updatable computed value to `IComputedValue<T, true>`.

docs/computeds.md

@@ -236,3 +236,8 @@ It is recommended to set this one to `true` on very expensive computed values. I
 ### `keepAlive`
 
 This avoids suspending computed values when they are not being observed by anything (see the above explanation). Can potentially create memory leaks, similar to the ones discussed for [reactions](reactions.md#always-dispose-of-reactions).
+
+### `set`
+
+This optional function allows the returned `IComputedValue<T>` to also act as something that knows how to update its backing data. When not provided, however, this function that is provided will throw an error.
+To help prevent runtime errors like this, especially when refactoring code from `IObservableValue<T>` to `IComputedValue<T>` the typescript types will are set up so that trying to call a function will result in a compile time error when this option is not set.

packages/mobx/__tests__/v5/base/typescript-tests.ts

@@ -2474,3 +2474,20 @@ test("observable.box should keep track of undefined and null in type", () => {
     const a = observable.box<string | undefined>()
     assert<IsExact<typeof a, IObservableValue<string | undefined>>>(true)
 })
+
+test("computed without a set function should not allow the consumer to set the value", () => {
+    const a = observable.box<string>("hello")
+    const b = computed(() => a.get())
+    assert<IsExact<typeof b, mobx.IComputedValue<string>>>(true)
+    // @ts-expect-error
+    b.set("world")
+})
+
+test("computed with a set function should allow the consumer to set the value", () => {
+    const a = observable.box<string>("hello")
+    const b = computed(() => a.get(), {
+        set: value => a.set(value)
+    })
+    assert<IsExact<typeof b, mobx.IComputedValue<string, true>>>(true)
+    b.set("world")
+})

packages/mobx/src/api/computed.ts

@@ -18,9 +18,11 @@ export const COMPUTED_STRUCT = "computed.struct"
 
 export interface IComputedFactory extends Annotation, PropertyDecorator {
     // @computed(opts)
-    <T>(options: IComputedValueOptions<T>): Annotation & PropertyDecorator
+    <T>(options: IComputedValueOptions<T, boolean>): Annotation & PropertyDecorator
     // computed(fn, opts)
-    <T>(func: () => T, options?: IComputedValueOptions<T>): IComputedValue<T>
+    <T>(func: () => T, options?: IComputedValueOptions<T, false>): IComputedValue<T, false>
+    <T>(func: () => T, options?: IComputedValueOptions<T, true>): IComputedValue<T, true>
+    <T>(func: () => T, options?: IComputedValueOptions<T, boolean>): IComputedValue<T, boolean>
 
     struct: Annotation & PropertyDecorator
 }

packages/mobx/src/core/computedvalue.ts

@@ -32,20 +32,19 @@ import {
     allowStateChangesEnd
 } from "../internal"
 
-export interface IComputedValue<T> {
+export interface IComputedValue<T, CanUpdate extends boolean = false> {
     get(): T
-    set(value: T): void
+    set: CanUpdate extends true ? (value: T) => void : undefined
 }
 
-export interface IComputedValueOptions<T> {
+export type IComputedValueOptions<T, CanUpdate extends boolean = false> = {
     get?: () => T
-    set?: (value: T) => void
     name?: string
     equals?: IEqualsComparer<T>
     context?: any
     requiresReaction?: boolean
     keepAlive?: boolean
-}
+} & (CanUpdate extends true ? { set: (value: T) => void } : { set?: undefined })
 
 export type IComputedDidChange<T = any> = {
     type: "update"
@@ -75,7 +74,7 @@ export type IComputedDidChange<T = any> = {
  *
  * If at any point it's outside batch and it isn't observed: reset everything and go to 1.
  */
-export class ComputedValue<T> implements IObservable, IComputedValue<T>, IDerivation {
+export class ComputedValue<T> implements IObservable, IComputedValue<T, boolean>, IDerivation {
     dependenciesState_ = IDerivationState_.NOT_TRACKING_
     observing_: IObservable[] = [] // nodes we are looking at. Our value depends on these nodes
     newObserving_ = null // during tracking it's an array with new observed observers
@@ -112,7 +111,7 @@ export class ComputedValue<T> implements IObservable, IComputedValue<T>, IDeriva
      * don't want to notify observers if it is structurally the same.
      * This is useful for working with vectors, mouse coordinates etc.
      */
-    constructor(options: IComputedValueOptions<T>) {
+    constructor(options: IComputedValueOptions<T, boolean>) {
         if (!options.get) {
             die(31)
         }

packages/mobx/src/mobx.ts

@@ -129,7 +129,6 @@ export {
     onReactionError,
     interceptReads as _interceptReads,
     IComputedValueOptions,
-    IActionRunInfo,
     _startAction,
     _endAction,
     allowStateReadsStart as _allowStateReadsStart,