Skip to content

Commit

Permalink
[undecorate] partial fix for #3460: @action => override (#3478)
Browse files Browse the repository at this point in the history
  • Loading branch information
@urugator
urugator authored Feb 9, 2023
1 parent cfcf3c6 commit c8f3b08
Show file tree
Hide file tree
Showing 7 changed files with 144 additions and 45 deletions.
5 changes: 5 additions & 0 deletions .changeset/many-steaks-eat.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"mobx-undecorate": minor
---

partial fix #3460: replace `action` with `override` if field uses override keyword
32 changes: 16 additions & 16 deletions docs/observable-state.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,22 +217,22 @@ Note that it is possible to pass `{ proxy: false }` as an option to `observable`

## Available annotations

| Annotation | Description |
| ---------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `observable`<br/>`observable.deep` | Defines a trackable field that stores state. If possible, any value assigned to `observable` is automatically converted to (deep) `observable`, [`autoAction`](#autoAction) or `flow` based on it's type. Only `plain object`, `array`, `Map`, `Set`, `function`, `generator function` are convertible. Class instances and others are untouched. |
| `observable.ref` | Like `observable`, but only reassignments will be tracked. The assigned values are completely ignored and will NOT be automatically converted to `observable`/[`autoAction`](#autoAction)/`flow`. For example, use this if you intend to store immutable data in an observable field. |
| `observable.shallow` | Like `observable.ref` but for collections. Any collection assigned will be made observable, but the contents of the collection itself won't become observable. |
| `observable.struct` | Like `observable`, except that any assigned value that is structurally equal to the current value will be ignored. |
| `action` | Mark a method as an action that will modify the state. Check out [actions](actions.md) for more details. Non-writable. |
| `action.bound` | Like action, but will also bind the action to the instance so that `this` will always be set. Non-writable. |
| `computed` | Can be used on a [getter](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/get) to declare it as a derived value that can be cached. Check out [computeds](computeds.md) for more details. |
| `computed.struct` | Like `computed`, except that if after recomputing the result is structurally equal to the previous result, no observers will be notified. |
| `true` | Infer the best annotation. Check out [makeAutoObservable](#makeautoobservable) for more details. |
| `false` | Explicitly do not annotate this property. |
| `flow` | Creates a `flow` to manage asynchronous processes. Check out [flow](actions.md#using-flow-instead-of-async--await-) for more details. Note that the inferred return type in TypeScript might be off. Non-writable. |
| `flow.bound` | Like flow, but will also bind the flow to the instance so that `this` will always be set. Non-writable. |
| `override` | [Applicable to inherited `action`, `flow`, `computed`, `action.bound` overridden by subclass](subclassing.md). |
| <span id="autoAction"></span> `autoAction` | Should not be used explicitly, but is used under the hood by `makeAutoObservable` to mark methods that can act as action or derivation, based on their calling context. It will be determined at runtime if the function is a derivation or action. |
| Annotation | Description |
| ------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `observable`<br/>`observable.deep` | Defines a trackable field that stores state. If possible, any value assigned to `observable` is automatically converted to (deep) `observable`, [`autoAction`](#autoAction) or `flow` based on it's type. Only `plain object`, `array`, `Map`, `Set`, `function`, `generator function` are convertible. Class instances and others are untouched. |
| `observable.ref` | Like `observable`, but only reassignments will be tracked. The assigned values are completely ignored and will NOT be automatically converted to `observable`/[`autoAction`](#autoAction)/`flow`. For example, use this if you intend to store immutable data in an observable field. |
| `observable.shallow` | Like `observable.ref` but for collections. Any collection assigned will be made observable, but the contents of the collection itself won't become observable. |
| `observable.struct` | Like `observable`, except that any assigned value that is structurally equal to the current value will be ignored. |
| `action` | Mark a method as an action that will modify the state. Check out [actions](actions.md) for more details. Non-writable. |
| `action.bound` | Like action, but will also bind the action to the instance so that `this` will always be set. Non-writable. |
| `computed` | Can be used on a [getter](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/get) to declare it as a derived value that can be cached. Check out [computeds](computeds.md) for more details. |
| `computed.struct` | Like `computed`, except that if after recomputing the result is structurally equal to the previous result, no observers will be notified. |
| `true` | Infer the best annotation. Check out [makeAutoObservable](#makeautoobservable) for more details. |
| `false` | Explicitly do not annotate this property. |
| `flow` | Creates a `flow` to manage asynchronous processes. Check out [flow](actions.md#using-flow-instead-of-async--await-) for more details. Note that the inferred return type in TypeScript might be off. Non-writable. |
| `flow.bound` | Like flow, but will also bind the flow to the instance so that `this` will always be set. Non-writable. |
| `override` | [Applicable to inherited `action`, `flow`, `computed`, `action.bound` overridden by subclass](subclassing.md). |
| <span id="autoAction"></span> `autoAction` | Should not be used explicitly, but is used under the hood by `makeAutoObservable` to mark methods that can act as action or derivation, based on their calling context. It will be determined at runtime if the function is a derivation or action. |

## Limitations

Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin-mobx/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ It's a bit opinionated and can lead to a lot of false positives depending on you
### mobx/no-anonymous-observer (deprecated)

_Deprecated in favor of [react/display-name](https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/display-name.md) + [componentWrapperFunctions](https://github.com/jsx-eslint/eslint-plugin-react). Example of **.eslintrc**:_

```
{
"rules": {
Expand All @@ -97,6 +98,7 @@ _Deprecated in favor of [react/display-name](https://github.com/jsx-eslint/eslin
}
}
```

---

Forbids anonymous functions or classes as `observer` components.
Expand Down
75 changes: 75 additions & 0 deletions packages/mobx-undecorate/__tests__/undecorate.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,81 @@ describe("action", () => {
}"
`)
})

test("method - override", () => {
expect(
convert(`
import { action } from "mobx"
class Box extends Shape {
constructor(arg) {
super(arg)
}
@action
override method(arg: number): boolean {
console.log('hi')
return true
}
}
`)
).toMatchInlineSnapshot(`
"import { action, override, makeObservable } from \\"mobx\\";
class Box extends Shape {
constructor(arg) {
super(arg)
makeObservable(this, {
method: override
});
}
override method(arg: number): boolean {
console.log('hi')
return true
}
}"
`)
})

test("method - override - keepDecorators", () => {
expect(
convert(
`
import { action } from "mobx"
class Box extends Shape {
constructor(arg) {
super(arg)
}
@action
override method(arg: number): boolean {
console.log('hi')
return true
}
}
`,
{ keepDecorators: true }
)
).toMatchInlineSnapshot(`
"import { action, override, makeObservable } from \\"mobx\\";
class Box extends Shape {
constructor(arg) {
super(arg)
makeObservable(this);
}
@override
override method(arg: number): boolean {
console.log('hi')
return true
}
}"
`)
})
})

describe("observable", () => {
Expand Down
1 change: 1 addition & 0 deletions packages/mobx-undecorate/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"homepage": "https://mobx.js.org/",
"dependencies": {
"@babel/core": "^7.9.0",
"@babel/parser": "^7.18.9",
"@babel/plugin-proposal-class-properties": "^7.8.3",
"@babel/plugin-proposal-decorators": "^7.8.3",
"@babel/plugin-transform-runtime": "^7.9.0",
Expand Down
20 changes: 18 additions & 2 deletions packages/mobx-undecorate/src/undecorate.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ export default function transform(
const lines = fileInfo.source.split("\n")
let changed = false
let needsInitializeImport = false
let importOverride = false
const decoratorsUsed = new Set<String>(options?.ignoreImports ? validDecorators : [])
let usesDecorate = options?.ignoreImports ? true : false
let hasReact = options?.ignoreImports ? true : false
Expand Down Expand Up @@ -228,6 +229,9 @@ export default function transform(
const { comments, ...k } = key
const { comments: comments2, ...v } = value
const prop = j.objectProperty(k, v)
if (v.name === "override") {
importOverride = true
}
prop.computed = !!computed
if (isPrivate) {
privates.push(k.name)
Expand Down Expand Up @@ -261,6 +265,9 @@ export default function transform(
if (!mobxImport.specifiers) {
mobxImport.specifiers = []
}
if (importOverride) {
mobxImport.specifiers.push(j.importSpecifier(j.identifier("override")))
}
mobxImport.specifiers.push(j.importSpecifier(j.identifier("makeObservable")))
}
}
Expand Down Expand Up @@ -308,7 +315,7 @@ export default function transform(
const exportDecl = j.exportDefaultDeclaration(newDefaultExportDefExpr.exported)

// re-create the class
let newClassDefExpr = j.classExpression(clazz.id, clazz.body, clazz.superClass)
const newClassDefExpr = j.classExpression(clazz.id, clazz.body, clazz.superClass)
newClassDefExpr.superTypeParameters = clazz.superTypeParameters
newClassDefExpr.typeParameters = clazz.typeParameters
newClassDefExpr.implements = clazz.implements
Expand Down Expand Up @@ -386,7 +393,7 @@ export default function transform(
if (!j.Decorator.check(decorator)) {
return property
}
const expr = decorator.expression
let expr = decorator.expression
if (j.Identifier.check(expr) && !decoratorsUsed.has(expr.name)) {
warn(`Found non-mobx decorator @${expr.name}`, decorator)
return property
Expand All @@ -400,6 +407,15 @@ export default function transform(
property.decorators.splice(0)
}

// Replace decorator with @override
if ((property as any).override) {
const overrideDecorator = j.decorator(j.identifier("override"))
if (options?.keepDecorators) {
property.decorators[0] = overrideDecorator
}
expr = overrideDecorator.expression
}

effects.membersMap.push([
property.key,
expr,
Expand Down
Loading

0 comments on commit c8f3b08

Please sign in to comment.