From f26830ae8436c2eee421460faa40afae06f4fba1 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 8 Jun 2026 11:37:28 +0700 Subject: [PATCH 1/2] chore(deps): bump changesets/action from 1.8.0 to 1.9.0 (#4664) Bumps [changesets/action](https://github.com/changesets/action) from 1.8.0 to 1.9.0. - [Release notes](https://github.com/changesets/action/releases) - [Changelog](https://github.com/changesets/action/blob/main/CHANGELOG.md) - [Commits](https://github.com/changesets/action/compare/v1.8.0...v1.9.0) --- updated-dependencies: - dependency-name: changesets/action dependency-version: 1.9.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index c3c16f7f0..522dda2fe 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -38,7 +38,7 @@ jobs: - name: Create Release Pull Request or Publish to npm id: changesets - uses: changesets/action@v1.8.0 + uses: changesets/action@v1.9.0 with: # This expects you to have a script called release which does a build for your packages and calls changeset publish publish: npm run release From 7fd93348e81f484c4ec2dcca8cde5fa6ed7412fe Mon Sep 17 00:00:00 2001 From: "Sergey S. Volkov" Date: Mon, 8 Jun 2026 11:50:00 +0300 Subject: [PATCH 2/2] Fix Stage 3 decorator inheritance (#4661) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: @computed override with super no longer causes cycle detection Fixes #4639 — when a child class @computed getter references super.value, MobX no longer throws 'Cycle detected in computation'. Architecture: - Common case (no inheritance): factories stored in lazyComputedKeys_ (identical to main branch), materialized on first read - Inheritance case: parent entries moved to computedGetterEntries_ (Map keyed by getter function), child entries in computedEntries_ - readComputed_ method checks derivation === expectedGet to detect super calls and routes to the correct ComputedValue - Prototype chain traversal + __mobx_get finds parent getter without per-instance WeakMap overhead - getObservablePropValue_ kept clean (no expectedGet) for makeObservable Performance vs main branch (50k instances × 10 computed getters): Heap: +0.5% Construct: -7.8% First-read: -4.2% Re-read: +6.6% (inherent cost of derivation check for super support) * Revert "fix: @computed override with super no longer causes cycle detection" This reverts commit a559575af2d7f71f311549c681a26efb3f63997e. * Fix Stage 3 computed inheritance * Simplify inheritance tests * Document unsupported (for now) test * Reorg tests --------- Co-authored-by: Gorbachev Egor <7gorbachevm@gmail.com> Co-authored-by: Сергей Волков --- .changeset/fresh-computed-super.md | 5 + docs/subclassing.md | 71 +++- .../stage3-decorators-inheritance.ts | 351 ++++++++++++++++++ packages/mobx/src/types/computedannotation.ts | 43 ++- 4 files changed, 453 insertions(+), 17 deletions(-) create mode 100644 .changeset/fresh-computed-super.md create mode 100644 packages/mobx/__tests__/decorators_20223/stage3-decorators-inheritance.ts diff --git a/.changeset/fresh-computed-super.md b/.changeset/fresh-computed-super.md new file mode 100644 index 000000000..ac32aa9d3 --- /dev/null +++ b/.changeset/fresh-computed-super.md @@ -0,0 +1,5 @@ +--- +"mobx": patch +--- + +Fix Stage 3 `@computed` overrides that delegate to a same-named parent getter via `super`. diff --git a/docs/subclassing.md b/docs/subclassing.md index 57610e393..349cd6f74 100644 --- a/docs/subclassing.md +++ b/docs/subclassing.md @@ -8,10 +8,13 @@ hide_title: true # Subclassing -Subclassing is supported with [limitations](#limitations). Most notably you can only **override actions/flows/computeds on prototype** - you cannot override _[field declarations](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes#field_declarations)_. Use the `override` annotation for methods/getters overridden in a subclass - see example below. Try to keep things simple and prefer composition over inheritance. +Subclassing is supported with [limitations](#limitations). Most notably you can only **override actions/flows/computeds on prototype** - you cannot override _[field declarations](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes#field_declarations)_. When using `makeObservable`, use the `override` annotation for methods/getters overridden in a subclass. When using modern decorators, redecorate the overridden prototype method/getter in the subclass. Try to keep things simple and prefer composition over inheritance. + + + ```javascript -import { makeObservable, observable, computed, action, override } from "mobx" +import { makeObservable, observable, computed, action, flow, override } from "mobx" class Parent { // Annotated instance fields are NOT overridable @@ -25,14 +28,16 @@ class Parent { action() {} actionBound() {} get computed() {} + *flow() {} constructor(value) { makeObservable(this, { observable: observable, - arrowAction: action + arrowAction: action, action: action, actionBound: action.bound, computed: computed, + flow: flow }) } } @@ -50,13 +55,15 @@ class Child extends Parent { action() {} actionBound() {} get computed() {} + *flow() {} /* --- NEW --- */ - childObservable = 0; + childObservable = 0 childArrowAction = () => {} childAction() {} childActionBound() {} get childComputed() {} + *childFlow() {} constructor(value) { super() @@ -65,17 +72,73 @@ class Child extends Parent { action: override, actionBound: override, computed: override, + flow: override, // new childObservable: observable, childArrowAction: action, childAction: action, childActionBound: action.bound, childComputed: computed, + childFlow: flow }) } } ``` + + +```javascript +import { observable, computed, action, flow } from "mobx" + +class Parent { + // Observable state fields are inherited, but should not be re-annotated + // or overridden by subclasses. + @observable accessor observable = 0 + + // Decorated instance fields should not be re-annotated or overridden. + @action arrowAction = () => {} + + // Non-decorated instance fields are overridable. + overridableArrowAction = action(() => {}) + + // Decorated prototype methods/getters are overridable. + @action action() {} + @action.bound actionBound() {} + @computed get computed() {} + @flow *flow() {} +} + +class Child extends Parent { + /* --- INHERITED --- */ + // Unsupported: do not re-annotate or override observable/accessor fields. + // @observable accessor observable = 5 + // @action arrowAction = () => {} + + // OK - not decorated + overridableArrowAction = action(() => {}) + + // OK - prototype + @action action() {} + @action.bound actionBound() {} + @computed get computed() { + return super.computed + } + @flow *flow() { + return yield super.flow() + } + + /* --- NEW --- */ + @observable accessor childObservable = 0 + @action childArrowAction = () => {} + @action childAction() {} + @action.bound childActionBound() {} + @computed get childComputed() {} + @flow *childFlow() {} +} +``` + + + ## Limitations 1. Only `action`, `computed`, `flow`, `action.bound` defined **on prototype** can be **overridden** by subclass. diff --git a/packages/mobx/__tests__/decorators_20223/stage3-decorators-inheritance.ts b/packages/mobx/__tests__/decorators_20223/stage3-decorators-inheritance.ts new file mode 100644 index 000000000..bbe58b32b --- /dev/null +++ b/packages/mobx/__tests__/decorators_20223/stage3-decorators-inheritance.ts @@ -0,0 +1,351 @@ +import { + action, + autorun, + computed, + flow, + flowResult, + isAction, + isComputedProp, + isFlow, + isObservableProp, + observable, + observe, + runInAction +} from "../../src/mobx" + +test("inherited observable accessor remains reactive in subclass", () => { + class Parent { + @observable accessor count = 1 + } + + class Child extends Parent {} + + const child = new Child() + const seen: number[] = [] + const dispose = autorun(() => seen.push(child.count)) + + runInAction(() => { + child.count = 2 + }) + dispose() + + expect(isObservableProp(child, "count")).toBe(true) + expect(seen).toEqual([1, 2]) +}) + +test("computed override can delegate to parent computed with super", () => { + class Parent { + @observable accessor count = 1 + + @computed + get number() { + return this.count + } + } + + class Child extends Parent { + @computed + override get number() { + return super.number + 1 + } + } + + const child = new Child() + + // Before first read, lazy computed bookkeeping must stay scoped to each requested key + expect(isObservableProp(child, "number")).toBe(true) + expect(isComputedProp(child, "number")).toBe(true) + expect(isObservableProp(child, "count")).toBe(true) + expect(isComputedProp(child, "count")).toBe(false) + expect(isObservableProp(child, "missing")).toBe(false) + expect(isComputedProp(child, "missing")).toBe(false) + + // Observing by public property key must observe the child computed, not the parent computed + const seen: number[] = [] + const dispose = observe(child, "number", change => seen.push(change.newValue), true) + + // Direct reads still delegate through super and remain reactive after materialization + expect(child.number).toBe(2) + runInAction(() => { + child.count = 2 + }) + dispose() + + expect(isObservableProp(child, "number")).toBe(true) + expect(isComputedProp(child, "number")).toBe(true) + expect(isObservableProp(child, "count")).toBe(true) + expect(isComputedProp(child, "count")).toBe(false) + expect(isObservableProp(child, "missing")).toBe(false) + expect(isComputedProp(child, "missing")).toBe(false) + expect(seen).toEqual([2, 3]) +}) + +test("computed override can delegate through multiple parent computeds", () => { + class GrandParent { + @observable accessor count = 1 + + @computed + get number() { + return this.count + } + } + + class Parent extends GrandParent { + @computed + override get number() { + return super.number + 1 + } + } + + class Child extends Parent { + @computed + override get number() { + return super.number + 1 + } + } + + const child = new Child() + const seen: number[] = [] + const dispose = observe(child, "number", change => seen.push(change.newValue), true) + + runInAction(() => { + child.count = 2 + }) + dispose() + + expect(child.number).toBe(4) + expect(seen).toEqual([3, 4]) +}) + +// #4660: A parent constructor read must not pin the parent computed as the child property +test("computed override wins when parent constructor reads the same key first", () => { + class Parent { + constructor() { + this.number + } + + @observable accessor count = 1 + + @computed + get number() { + return this.count + } + } + + class Child extends Parent { + @computed + override get number() { + return 0 + } + } + + const child = new Child() + const seen: number[] = [] + const dispose = observe(child, "number", change => seen.push(change.newValue), true) + dispose() + + expect(child.number).toBe(0) + expect(isComputedProp(child, "number")).toBe(true) + expect(seen).toEqual([0]) +}) + +test("manually wrapped action field can be overridden as an ordinary field", () => { + class Parent { + @observable accessor count = 0 + + increment = action(() => { + this.count += 1 + }) + } + + class Child extends Parent { + increment = action(() => { + this.count += 2 + }) + } + + const child = new Child() + const seen: number[] = [] + const dispose = autorun(() => seen.push(child.count)) + + child.increment() + dispose() + + expect(isAction(child.increment)).toBe(true) + expect(seen).toEqual([0, 2]) +}) + +test("subclass can add new observable, computed, and action members", () => { + class Parent { + @observable accessor count = 1 + } + + class Child extends Parent { + @observable accessor extra = 2 + + @computed + get total() { + return this.count + this.extra + } + + @action + incrementExtra() { + this.extra += 1 + } + + @action.bound + incrementCount() { + this.count += 1 + } + } + + const child = new Child() + const seen: number[] = [] + const dispose = autorun(() => seen.push(child.total)) + const incrementCount = child.incrementCount + + child.incrementExtra() + incrementCount() + dispose() + + expect(isComputedProp(child, "total")).toBe(true) + expect(isAction(child.incrementExtra)).toBe(true) + expect(isAction(child.incrementCount)).toBe(true) + expect(seen).toEqual([3, 4, 5]) +}) + +test("action override can call parent action with super", () => { + class Parent { + @observable accessor count = 0 + + @action + increment(value: number) { + this.count += value + } + } + + class Child extends Parent { + @action + override increment(value: number) { + super.increment(value) + this.count += 1 + } + } + + const child = new Child() + const seen: number[] = [] + const dispose = autorun(() => seen.push(child.count)) + + child.increment(2) + dispose() + + expect(isAction(child.increment)).toBe(true) + expect(child.count).toBe(3) + expect(seen).toEqual([0, 3]) +}) + +test("action.bound override can be called after extraction", () => { + class Parent { + @observable accessor count = 0 + + @action.bound + increment(value: number) { + this.count += value + } + } + + class Child extends Parent { + @action.bound + override increment(value: number) { + super.increment(value) + this.count += 1 + } + } + + const child = new Child() + const seen: number[] = [] + const dispose = autorun(() => seen.push(child.count)) + const increment = child.increment + + increment(2) + dispose() + + expect(isAction(child.increment)).toBe(true) + expect(child.count).toBe(3) + expect(seen).toEqual([0, 3]) +}) + +test("flow override can wait for parent flow before child updates", async () => { + class ProfileStore { + @observable accessor status: "idle" | "loading" | "ready" = "idle" + @observable accessor name = ""; + + @flow + *loadProfile(userId: number) { + this.status = "loading" + yield Promise.resolve() + this.name = `User ${userId}` + this.status = "ready" + } + } + + class ProfilePageStore extends ProfileStore { + @observable accessor title = ""; + + @flow + override *loadProfile(userId: number) { + yield flowResult(super.loadProfile(userId)) + this.title = `${this.name} profile` + } + } + + const profile = new ProfilePageStore() + const seen: string[] = [] + const dispose = autorun(() => seen.push(`${profile.status}|${profile.name}|${profile.title}`)) + + await flowResult(profile.loadProfile(2)) + dispose() + + expect(isFlow(profile.loadProfile)).toBe(true) + expect(profile.status).toBe("ready") + expect(profile.name).toBe("User 2") + expect(profile.title).toBe("User 2 profile") + expect(seen).toContain("idle||") + expect(seen).toContain("loading||") + expect(seen[seen.length - 1]).toBe("ready|User 2|User 2 profile") +}) + +// #4660: If a subclass has a same-named @computed override that is still lazy, +// and another subclass member calls super.foo before foo itself has been read, +// this call materializes the child lazy entry and returns the overridden value +// instead of the parent value. For example, with override get number() { +// return super.number + 1 } and get parentNumber() { return super.number }, +// the first child.parentNumber reads as 2 instead of the parent 1, making super +// behavior order-dependent +test.skip("computed sibling can read parent computed with super before child computed is read", () => { + class Parent { + @observable accessor count = 1 + + @computed + get number() { + return this.count + } + } + + class Child extends Parent { + @computed + override get number() { + return super.number + 1 + } + + @computed + get parentNumber() { + return super.number + } + } + + const child = new Child() + + expect(child.parentNumber).toBe(1) + expect(child.number).toBe(2) +}) diff --git a/packages/mobx/src/types/computedannotation.ts b/packages/mobx/src/types/computedannotation.ts index 5426af70a..11dd4e90a 100644 --- a/packages/mobx/src/types/computedannotation.ts +++ b/packages/mobx/src/types/computedannotation.ts @@ -53,28 +53,45 @@ function decorate_20223_(this: Annotation, get, context: ClassGetterDecoratorCon } const ann = this const { name: key, addInitializer } = context + let computedValues: WeakMap> | undefined // Defer ComputedValue creation until first access — avoids allocating // ComputedValues for getters that are never read on a given instance. // The factory is materialised by ObservableObjectAdministration on demand. + function createComputedValue(target: object, adm: ObservableObjectAdministration) { + const options = { + ...ann.options_, + get, + context: target + } + options.name ||= __DEV__ + ? `${adm.name_}.${key.toString()}` + : `ObservableObject.${key.toString()}` + return new ComputedValue(options) + } + addInitializer(function () { const adm: ObservableObjectAdministration = asObservableObject(this)[$mobx] - const target = this - ;(adm.lazyComputedKeys_ ??= new Map()).set(key, () => { - const options = { - ...ann.options_, - get, - context: target - } - options.name ||= __DEV__ - ? `${adm.name_}.${key.toString()}` - : `ObservableObject.${key.toString()}` - return new ComputedValue(options) - }) + const target = this as object + const observable = adm.values_.get(key) + if (observable instanceof ComputedValue && observable.derivation !== get) { + adm.values_.delete(key) + } + ;(adm.lazyComputedKeys_ ??= new Map()).set(key, () => createComputedValue(target, adm)) }) return function () { - return this[$mobx].getObservablePropValue_(key) + const adm: ObservableObjectAdministration = this[$mobx] + const observable = adm.values_.get(key) + if (observable instanceof ComputedValue && observable.derivation !== get) { + let computed = computedValues?.get(this) + if (!computed) { + computed = createComputedValue(this, adm) + ;(computedValues ??= new WeakMap()).set(this, computed) + } + return computed.get() + } + return adm.getObservablePropValue_(key) } }