Skip to content

Commit 7fd9334

Browse files
js2mekubkСергей Волков
authored
Fix Stage 3 decorator inheritance (mobxjs#4661)
* fix: @computed override with super no longer causes cycle detection Fixes mobxjs#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 a559575. * 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: Сергей Волков <ser.volkov@vk.team>
1 parent f26830a commit 7fd9334

4 files changed

Lines changed: 453 additions & 17 deletions

File tree

.changeset/fresh-computed-super.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"mobx": patch
3+
---
4+
5+
Fix Stage 3 `@computed` overrides that delegate to a same-named parent getter via `super`.

docs/subclassing.md

Lines changed: 67 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,13 @@ hide_title: true
88

99
# Subclassing
1010

11-
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.
11+
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.
12+
13+
<!--DOCUSAURUS_CODE_TABS-->
14+
<!--makeObservable-->
1215

1316
```javascript
14-
import { makeObservable, observable, computed, action, override } from "mobx"
17+
import { makeObservable, observable, computed, action, flow, override } from "mobx"
1518

1619
class Parent {
1720
// Annotated instance fields are NOT overridable
@@ -25,14 +28,16 @@ class Parent {
2528
action() {}
2629
actionBound() {}
2730
get computed() {}
31+
*flow() {}
2832

2933
constructor(value) {
3034
makeObservable(this, {
3135
observable: observable,
32-
arrowAction: action
36+
arrowAction: action,
3337
action: action,
3438
actionBound: action.bound,
3539
computed: computed,
40+
flow: flow
3641
})
3742
}
3843
}
@@ -50,13 +55,15 @@ class Child extends Parent {
5055
action() {}
5156
actionBound() {}
5257
get computed() {}
58+
*flow() {}
5359

5460
/* --- NEW --- */
55-
childObservable = 0;
61+
childObservable = 0
5662
childArrowAction = () => {}
5763
childAction() {}
5864
childActionBound() {}
5965
get childComputed() {}
66+
*childFlow() {}
6067

6168
constructor(value) {
6269
super()
@@ -65,17 +72,73 @@ class Child extends Parent {
6572
action: override,
6673
actionBound: override,
6774
computed: override,
75+
flow: override,
6876
// new
6977
childObservable: observable,
7078
childArrowAction: action,
7179
childAction: action,
7280
childActionBound: action.bound,
7381
childComputed: computed,
82+
childFlow: flow
7483
})
7584
}
7685
}
7786
```
7887

88+
<!--Modern decorators-->
89+
90+
```javascript
91+
import { observable, computed, action, flow } from "mobx"
92+
93+
class Parent {
94+
// Observable state fields are inherited, but should not be re-annotated
95+
// or overridden by subclasses.
96+
@observable accessor observable = 0
97+
98+
// Decorated instance fields should not be re-annotated or overridden.
99+
@action arrowAction = () => {}
100+
101+
// Non-decorated instance fields are overridable.
102+
overridableArrowAction = action(() => {})
103+
104+
// Decorated prototype methods/getters are overridable.
105+
@action action() {}
106+
@action.bound actionBound() {}
107+
@computed get computed() {}
108+
@flow *flow() {}
109+
}
110+
111+
class Child extends Parent {
112+
/* --- INHERITED --- */
113+
// Unsupported: do not re-annotate or override observable/accessor fields.
114+
// @observable accessor observable = 5
115+
// @action arrowAction = () => {}
116+
117+
// OK - not decorated
118+
overridableArrowAction = action(() => {})
119+
120+
// OK - prototype
121+
@action action() {}
122+
@action.bound actionBound() {}
123+
@computed get computed() {
124+
return super.computed
125+
}
126+
@flow *flow() {
127+
return yield super.flow()
128+
}
129+
130+
/* --- NEW --- */
131+
@observable accessor childObservable = 0
132+
@action childArrowAction = () => {}
133+
@action childAction() {}
134+
@action.bound childActionBound() {}
135+
@computed get childComputed() {}
136+
@flow *childFlow() {}
137+
}
138+
```
139+
140+
<!--END_DOCUSAURUS_CODE_TABS-->
141+
79142
## Limitations
80143

81144
1. Only `action`, `computed`, `flow`, `action.bound` defined **on prototype** can be **overridden** by subclass.

0 commit comments

Comments
 (0)