Merge branch 'main' into decorators2022

.changeset/polite-laws-provide.md

@@ -0,0 +1,5 @@
+---
+"mobx-react": patch
+---
+
+fix #3730: class component does not react to state changes performed before mount

.github/workflows/coveralls.yml

@@ -28,6 +28,6 @@ jobs:
         run: yarn coverage
 
       - name: Upload to coveralls
-        uses: coverallsapp/github-action@v2.1.2 
+        uses: coverallsapp/github-action@v2.2.1 
         with:
           github-token: ${{ secrets.GITHUB_TOKEN }}

.github/workflows/release.yml

@@ -11,7 +11,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - name: Checkout Repo
-        uses: actions/checkout@master
+        uses: actions/checkout@main
         with:
           # This makes Actions fetch all Git history so that Changesets can generate changelogs with the correct commits
           fetch-depth: 0
@@ -34,4 +34,4 @@ jobs:
           title: Next release
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-          NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
+          NPM_TOKEN: ${{ secrets.NPM_TOKEN }}

docs/README.md

@@ -42,6 +42,7 @@ MobX is made possible by the generosity of the sponsors below, and many other [i
 <a href="https://casinosites.ltd.uk/?utm_source=sponsorship&utm_medium=mobx&utm_campaign=readme"><img src="https://mobx.js.org/assets/casino2.png" align="center" width="80" title="Casino Sites" alt="Casino Sites"/></a>
 <a href="https://upper.co/?utm_source=github_mobxjs_sponsorship&utm_medium=paid_acquisition&utm_campaign=sponsorship"><img src="https://mobx.js.org/assets/upper.png" align="center" width="80" title="UPPER" alt="UPPER"/></a>
 <a href="https://www.easeus.com/?utm_source=github_mobxjs_sponsorship&utm_medium=readme&utm_campaign=sponsorship"><img src="https://mobx.js.org/assets/easeus.png" align="center" width="80" title="EaseUS" alt="EaseUS"/></a>
+<a href="https://rxdb.info/?utm_source=github_mobxjs_sponsorship&utm_medium=readme&utm_campaign=sponsorship"><img src="https://mobx.js.org/assets/rxdb.png" align="center" width="80" title="RxDB" alt="RxDB JavaScript Database"/></a>
 
 **🥉 Bronze sponsors (\$500+ total contributions):**<br/>
 <a href="https://www.bugsnag.com/platforms/react-error-reporting?utm_source=MobX&utm_medium=Website&utm_content=open-source&utm_campaign=2019-community&utm_term=20190913"><img src="https://mobx.js.org/assets/bugsnag.jpg" align="center" width="80" title="Bugsnag" alt="Bugsnag"/></a>

docs/observable-state.md

@@ -260,7 +260,7 @@ The above APIs take an optional `options` argument which is an object that suppo
 -   **`autoBind: true`** uses `action.bound`/`flow.bound` by default, rather than `action`/`flow`. Does not affect explicitely annotated members.
 -   **`deep: false`** uses `observable.ref` by default, rather than `observable`. Does not affect explicitely annotated members.
 -   **`name: <string>`** gives the object a debug name that is printed in error messages and reflection APIs.
--   **`proxy: false`** forces `observable(thing)` to use non-[**proxy**](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy) implementation. This is a good option if the shape of the object will not change over time, as non-proxied objects are easier to debug and faster. See [avoiding proxies](#avoid-proxies).
+-   **`proxy: false`** forces `observable(thing)` to use non-[**proxy**](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy) implementation. This is a good option if the shape of the object will not change over time, as non-proxied objects are easier to debug and faster. This option is **not** available for `make(Auto)Observable`, see [avoiding proxies](#avoid-proxies).
 
 <details id="one-options-per-target"><summary>**Note:** options are *sticky* and can be provided only once<a href="#one-options-per-target" class="tip-anchor"></a></summary>
 `options` argument can be provided only for `target` that is NOT observable yet.<br>
@@ -283,7 +283,7 @@ const plainMap = new Map(observableMap)
 ```
 
 To convert a data tree recursively to plain objects, the [`toJS`](api.md#tojs) utility can be used.
-For classes, it is recommend to implement a `toJSON()` method, as it will be picked up by `JSON.stringify`.
+For classes, it is recommended to implement a `toJSON()` method, as it will be picked up by `JSON.stringify`.
 
 ## A short note on classes
 

docs/react-integration.md

@@ -349,7 +349,7 @@ const TimerView = observer(
 )
 ```
 
-Check out [mobx-react docs](https://github.com/mobxjs/mobx-react#api-documentation) for more information.
+Check out [mobx-react docs](https://github.com/mobxjs/mobx/tree/main/packages/mobx-react#class-components) for more information.
 
 </details>
 

docs/reactions.md

@@ -364,9 +364,10 @@ Number of milliseconds that can be used to throttle the effect function. If zero
 
 Set a limited amount of time that `when` will wait for. If the deadline passes, `when` will reject / throw.
 
-### `signal` _(when)_
+### `signal`
 
-An AbortSignal object instance; allows you to abort waiting for the reaction via an AbortController. This will also cause the returned promise to reject with an error "WHEN_ABORTED". This option is ignored when using an effect function, and only applies with the promised based version.
+An AbortSignal object instance; can be used as an alternative method for disposal.<br>
+When used with promise version of `when`, the promise rejects with the "WHEN_ABORTED" error.
 
 ### `onError`
 

packages/eslint-plugin-mobx/package.json

@@ -47,6 +47,6 @@
     "scripts": {
         "test": "jest",
         "build": "yarn rollup --config",
-        "prepublish": "yarn build"
+        "prepublishOnly": "yarn build"
     }
 }

packages/mobx-react-lite/CHANGELOG.md

@@ -1,5 +1,15 @@
 # mobx-react-lite
 
+## 4.0.3
+
+### Patch Changes
+
+-   [`58bb052c`](https://github.com/mobxjs/mobx/commit/58bb052ca41b8592e5bd5c3003b68ec52da53f33) [#3670](https://github.com/mobxjs/mobx/pull/3670) Thanks [@urugator](https://github.com/urugator)! - fix #3669: SSR: `useSyncExternalStore` throws due to missing `getServerSnapshot`
+
+*   [`473cb3f5`](https://github.com/mobxjs/mobx/commit/473cb3f5fc8bf43abdd1c9c7857fe2820d2291fe) [#3718](https://github.com/mobxjs/mobx/pull/3718) Thanks [@mweststrate](https://github.com/mweststrate)! - - Fixed `observer` in `StrictMode` #3671
+    -   **[BREAKING CHANGE]** Class component's `props`/`state`/`context` are no longer observable. Attempt to use these in any derivation other than component's `render` throws and error. For details see https://github.com/mobxjs/mobx/blob/main/packages/mobx-react/README.md#note-on-using-props-and-state-in-derivations
+    -   Extending or applying `observer` classes is now explicitly forbidden
+
 ## 4.0.2
 
 ### Patch Changes

packages/mobx-react-lite/__tests__/observer.test.tsx

@@ -1087,3 +1087,23 @@ test("Anonymous component displayName #3192", () => {
     expect(observerError.message).toEqual(memoError.message)
     consoleErrorSpy.mockRestore()
 })
+
+test("StrictMode #3671", async () => {
+    const o = mobx.observable({ x: 0 })
+
+    const Cmp = observer(() => o.x as any)
+
+    const { container, unmount } = render(
+        <React.StrictMode>
+            <Cmp />
+        </React.StrictMode>
+    )
+
+    expect(container).toHaveTextContent("0")
+    act(
+        mobx.action(() => {
+            o.x++
+        })
+    )
+    expect(container).toHaveTextContent("1")
+})

packages/mobx-react-lite/__tests__/strictAndConcurrentModeUsingFinalizationRegistry.test.tsx

@@ -24,6 +24,7 @@ async function gc_cycle() {
 }
 
 test("uncommitted components should not leak observations", async () => {
+    jest.setTimeout(30_000)
     const store = mobx.observable({ count1: 0, count2: 0 })
 
     // Track whether counts are observed
@@ -52,7 +53,6 @@ test("uncommitted components should not leak observations", async () => {
 
     // Allow gc to kick in in case to let finalization registry cleanup
     await gc_cycle()
-
     // Can take a while (especially on CI) before gc actually calls the registry
     await waitFor(
         () => {
@@ -62,7 +62,7 @@ test("uncommitted components should not leak observations", async () => {
             expect(count2IsObserved).toBeFalsy()
         },
         {
-            timeout: 3000,
+            timeout: 10_000,
             interval: 200
         }
     )

packages/mobx-react-lite/__tests__/useAsObservableSource.deprecated.test.tsx

@@ -126,19 +126,19 @@ describe("base useAsObservableSource should work", () => {
         const { container } = render(<Parent />)
 
         expect(container.querySelector("span")!.innerHTML).toBe("10")
-        expect(counterRender).toBe(1)
+        expect(counterRender).toBe(2)
 
         act(() => {
             ;(container.querySelector("#inc")! as any).click()
         })
         expect(container.querySelector("span")!.innerHTML).toBe("11")
-        expect(counterRender).toBe(2)
+        expect(counterRender).toBe(3)
 
         act(() => {
             ;(container.querySelector("#incmultiplier")! as any).click()
         })
         expect(container.querySelector("span")!.innerHTML).toBe("22")
-        expect(counterRender).toBe(4) // TODO: should be 3
+        expect(counterRender).toBe(5) // TODO: should be 3
     })
 })
 
@@ -271,7 +271,7 @@ describe("combining observer with props and stores", () => {
             store.x = 10
         })
 
-        expect(renderedValues).toEqual([10, 15, 15, 20]) // TODO: should have one 15 less
+        expect(renderedValues).toEqual([10, 10, 15, 15, 20]) // TODO: should have one 15 less
 
         // TODO: re-enable this line. When debugging, the correct value is returned from render,
         // which is also visible with renderedValues, however, querying the dom doesn't show the correct result

packages/mobx-react-lite/__tests__/useAsObservableSource.test.tsx

@@ -192,19 +192,19 @@ describe("base useAsObservableSource should work", () => {
         const { container } = render(<Parent />)
 
         expect(container.querySelector("span")!.innerHTML).toBe("10")
-        expect(counterRender).toBe(1)
+        expect(counterRender).toBe(2)
 
         act(() => {
             ;(container.querySelector("#inc")! as any).click()
         })
         expect(container.querySelector("span")!.innerHTML).toBe("11")
-        expect(counterRender).toBe(2)
+        expect(counterRender).toBe(3)
 
         act(() => {
             ;(container.querySelector("#incmultiplier")! as any).click()
         })
         expect(container.querySelector("span")!.innerHTML).toBe("22")
-        expect(counterRender).toBe(4) // TODO: should be 3
+        expect(counterRender).toBe(5) // TODO: should be 3
     })
 })
 
@@ -337,7 +337,7 @@ describe("combining observer with props and stores", () => {
             store.x = 10
         })
 
-        expect(renderedValues).toEqual([10, 15, 15, 20]) // TODO: should have one 15 less
+        expect(renderedValues).toEqual([10, 10, 15, 15, 20]) // TODO: should have one 15 less
 
         expect(container.querySelector("div")!.textContent).toBe("20")
     })

packages/mobx-react-lite/__tests__/useLocalObservable.test.tsx

@@ -116,7 +116,7 @@ describe("is used to keep observable within component body", () => {
         fireEvent.click(div)
         expect(div.textContent).toBe("3-2")
 
-        expect(renderCount).toBe(3)
+        expect(renderCount).toBe(4)
     })
 
     it("actions can be used", () => {
@@ -418,19 +418,19 @@ describe("is used to keep observable within component body", () => {
             const { container } = render(<Parent />)
 
             expect(container.querySelector("span")!.innerHTML).toBe("10")
-            expect(counterRender).toBe(1)
+            expect(counterRender).toBe(2)
 
             act(() => {
                 ;(container.querySelector("#inc")! as any).click()
             })
             expect(container.querySelector("span")!.innerHTML).toBe("11")
-            expect(counterRender).toBe(2)
+            expect(counterRender).toBe(3)
 
             act(() => {
                 ;(container.querySelector("#incmultiplier")! as any).click()
             })
             expect(container.querySelector("span")!.innerHTML).toBe("22")
-            expect(counterRender).toBe(4) // TODO: should be 3
+            expect(counterRender).toBe(5) // TODO: should be 3
         })
     })
 })

packages/mobx-react-lite/__tests__/useLocalStore.deprecated.test.tsx

@@ -119,8 +119,8 @@ describe("is used to keep observable within component body", () => {
         fireEvent.click(div)
         expect(div.textContent).toBe("3-2")
 
-        // though render 3 times, mobx.observable only called once
-        expect(renderCount).toBe(3)
+        // though render 4 times, mobx.observable only called once
+        expect(renderCount).toBe(4)
     })
 
     it("actions can be used", () => {
@@ -424,19 +424,19 @@ describe("is used to keep observable within component body", () => {
             const { container } = render(<Parent />)
 
             expect(container.querySelector("span")!.innerHTML).toBe("10")
-            expect(counterRender).toBe(1)
+            expect(counterRender).toBe(2)
 
             act(() => {
                 ;(container.querySelector("#inc")! as any).click()
             })
             expect(container.querySelector("span")!.innerHTML).toBe("11")
-            expect(counterRender).toBe(2)
+            expect(counterRender).toBe(3)
 
             act(() => {
                 ;(container.querySelector("#incmultiplier")! as any).click()
             })
             expect(container.querySelector("span")!.innerHTML).toBe("22")
-            expect(counterRender).toBe(4) // TODO: should be 3
+            expect(counterRender).toBe(5) // TODO: should be 3
         })
     })
 })

packages/mobx-react-lite/package.json

@@ -1,6 +1,6 @@
 {
     "name": "mobx-react-lite",
-    "version": "4.0.2",
+    "version": "4.0.3",
     "description": "Lightweight React bindings for MobX based on React 16.8+ and Hooks",
     "source": "src/index.ts",
     "main": "dist/index.js",
@@ -76,6 +76,6 @@
         "test:size": "yarn import-size --report . observer useLocalObservable",
         "test:types": "tsc --noEmit",
         "test:check": "yarn test:types",
-        "prepublish": "cd ../mobx && yarn build --target publish && cd ../mobx-react-lite && yarn build --target publish && yarn build:cjs && yarn build:es"
+        "prepublishOnly": "cd ../mobx && yarn build --target publish && cd ../mobx-react-lite && yarn build --target publish && yarn build:cjs && yarn build:es"
     }
 }

packages/mobx-react-lite/src/useLocalStore.ts

@@ -13,10 +13,11 @@ export function useLocalStore<TStore extends Record<string, any>, TSource extend
     initializer: (source?: TSource) => TStore,
     current?: TSource
 ): TStore {
-    if ("production" !== process.env.NODE_ENV)
+    if ("production" !== process.env.NODE_ENV) {
         useDeprecated(
             "[mobx-react-lite] 'useLocalStore' is deprecated, use 'useLocalObservable' instead."
         )
+    }
     const source = current && useAsObservableSource(current)
     return useState(() => observable(initializer(source), undefined, { autoBind: true }))[0]
 }

packages/mobx-react-lite/src/useObserver.ts

@@ -12,7 +12,7 @@ const getServerSnapshot = () => {}
 // otherwise it will prevent GC and therefore reaction disposal via FinalizationRegistry.
 type ObserverAdministration = {
     reaction: Reaction | null // also serves as disposed flag
-    forceUpdate: Function | null // also serves as mounted flag
+    onStoreChange: Function | null // also serves as mounted flag
     // BC: we will use local state version if global isn't available.
     // It should behave as previous implementation - tearing is still present,
     // because there is no cross component synchronization,
@@ -27,18 +27,18 @@ type ObserverAdministration = {
 const mobxGlobalState = _getGlobalState()
 
 // BC
-const globalStateVersionIsAvailable = typeof mobxGlobalState.globalVersion !== "undefined"
+const globalStateVersionIsAvailable = typeof mobxGlobalState.stateVersion !== "undefined"
 
 function createReaction(adm: ObserverAdministration) {
     adm.reaction = new Reaction(`observer${adm.name}`, () => {
         if (!globalStateVersionIsAvailable) {
             // BC
             adm.stateVersion = Symbol()
         }
-        // Force update won't be avaliable until the component "mounts".
+        // onStoreChange won't be available until the component "mounts".
         // If state changes in between initial render and mount,
         // `useSyncExternalStore` should handle that by checking the state version and issuing update.
-        adm.forceUpdate?.()
+        adm.onStoreChange?.()
     })
 }
 
@@ -49,28 +49,34 @@ export function useObserver<T>(render: () => T, baseComponentName: string = "obs
 
     const admRef = React.useRef<ObserverAdministration | null>(null)
 
+    // Provides ability to force component update without changing state version
+    const [, forceUpdate] = React.useState<Symbol>()
+
     if (!admRef.current) {
         // First render
         const adm: ObserverAdministration = {
             reaction: null,
-            forceUpdate: null,
+            onStoreChange: null,
             stateVersion: Symbol(),
             name: baseComponentName,
             subscribe(onStoreChange: () => void) {
                 // Do NOT access admRef here!
                 observerFinalizationRegistry.unregister(adm)
-                adm.forceUpdate = onStoreChange
+                adm.onStoreChange = onStoreChange
                 if (!adm.reaction) {
-                    // We've lost our reaction and therefore all subscriptions.
+                    // We've lost our reaction and therefore all subscriptions, occurs when:
+                    // 1. Timer based finalization registry disposed reaction before component mounted.
+                    // 2. React "re-mounts" same component without calling render in between (typically <StrictMode>).
                     // We have to recreate reaction and schedule re-render to recreate subscriptions,
                     // even if state did not change.
                     createReaction(adm)
-                    adm.forceUpdate()
+                    // `onStoreChange` won't force update if subsequent `getSnapshot` returns same value.
+                    forceUpdate(Symbol())
                 }
 
                 return () => {
                     // Do NOT access admRef here!
-                    adm.forceUpdate = null
+                    adm.onStoreChange = null
                     adm.reaction?.dispose()
                     adm.reaction = null
                 }