Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement 2022.3/Stage 3 Decorator Support #3638

Closed
wants to merge 11 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/afraid-cooks-nail.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"mobx": minor
---

Add 2022.3 Decorators support
1 change: 1 addition & 0 deletions .github/workflows/build_and_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ on:
branches: [main]
pull_request:
branches: [main]
workflow_dispatch: {}

jobs:
build:
Expand Down
74 changes: 65 additions & 9 deletions docs/enabling-decorators.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,71 @@ hide_title: true

<script async type="text/javascript" src="//cdn.carbonads.com/carbon.js?serve=CEBD4KQ7&placement=mobxjsorg" id="_carbonads_js"></script>

# Enabling decorators {🚀}
# Enabling Decorators

MobX before version 6 encouraged the use of ES.next decorators to mark things as `observable`, `computed` and `action`. However, decorators are currently not an ES standard, and the process of standardization is taking a long time. It also looks like the standard will be different from the way decorators were implemented previously. In the interest of compatibility we have chosen to move away from them in MobX 6, and recommend the use of [`makeObservable` / `makeAutoObservable`](observable-state.md) instead.
After years of alterations, ES decorators have finally reached Stage 3 in the TC39 process, meaning that they are quite stable and won't undergo breaking changes again like the previous decorator proposals have. MobX has implemented support for this new "2022.3/Stage 3" decorator syntax. If you're looking for information on "legacy" decorators, look below or refer to older versions of the documentation.
Matchlighter marked this conversation as resolved.
Show resolved Hide resolved

But many existing codebases use decorators, and a lot of the documentation and tutorial material online uses them as well. The rule is that anything you can use as an annotation to `makeObservable`, such as `observable`, `action` and `computed`, you can also use as a decorator. So let's examine what that looks like:
2022.3 Decorators are supported in Babel (see https://babeljs.io/docs/babel-plugin-proposal-decorators) and in TypeScript (5.0+).

## Usage

```javascript
import { observable, computed, action } from "mobx"

class Todo {
mweststrate marked this conversation as resolved.
Show resolved Hide resolved
id = Math.random()
@observable accessor title = ""
@observable accessor finished = false

@action
toggle() {
this.finished = !this.finished
}
}

class TodoList {
@observable accessor todos = []

@computed
get unfinishedTodoCount() {
return this.todos.filter(todo => !todo.finished).length
}
}
```

Notice the usage of the new `accessor` keyword when using `@observable`.
It is part of the 2022.3 spec and is required if you want to use decorators without `makeObservable()`.
That being said, you _can_ do without it if you continue to call `makeObservable()` like so:

```javascript
class Todo {
@observable title = ""

constructor() {
makeObservable(this)
}
}
```

## Changes/Gotchas

MobX' 2022.3 Decorators are very similar to the pre-MobX 6 decorators, so usage is mostly the same, but there are some gotchas:

- `@observable accessor` decorators are _not_ enumerable. `accessor`s do not have a direct equivalent in the past - they're a new concept in the language. We've chosen to make them non-enumerable, non-own properties in order to better follow the spirit of the ES language and what `accessor` means.
mweststrate marked this conversation as resolved.
Show resolved Hide resolved
The main cases for enumerability seem to have been around serialization and rest destructuring.
- Regarding serialization, implicitly serializing all properties probably isn't ideal in an OOP-world anyway, so this doesn't seem like a substantial issue (consider implementing `toJSON` or using `serializr` as possible alternatives)
- Addressing rest-destructuring, such is an anti-pattern in MobX - doing so would (likely unwantedly) touch all observables and make the observer overly-reactive).
- `@action some_field = () => {}` was and is valid usage (_if_ `makeObservable()` is also used). However, `@action accessor some_field = () => {}` is never valid.

<details id="legacy-decorators"><summary>Legacy Decorators<a href="#legacy-decorators" class="tip-anchor"></a></summary>

We do not recommend new codebases that use MobX use legacy decorators until the point when they become an official part of the language, but you can still use them. It does require setup for transpilation so you have to use Babel or TypeScript.

</details>

## MobX Core decorators {🚀}

MobX before version 6 encouraged the use of ES.next decorators to mark things as `observable`, `computed` and `action`. While MobX 6 recommends against using these decorators (and instead using [`makeObservable` / `makeAutoObservable`](observable-state.md)), it is still possible.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a bit confusing. Are they recommended for use now or not? Maybe this section should be titled "legacy decorators" and the text changed?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call! I'll went to the docs and there is still some more straightening out to do, but we're close!


```javascript
import { makeObservable, observable, computed, action } from "mobx"
Expand Down Expand Up @@ -52,7 +112,7 @@ When migrating from MobX 4/5 to 6, we recommend to always run the code-mod, to m

Check out the [Migrating from MobX 4/5 {🚀}](migrating-from-4-or-5.md) section.

## Using `observer` as a decorator
### Using `observer` as a decorator

The `observer` function from `mobx-react` is both a function and a decorator that can be used on class components:

Expand All @@ -63,10 +123,6 @@ class Timer extends React.Component {
}
```

## How to enable decorator support

We do not recommend new codebases that use MobX use decorators until the point when they become an official part of the language, but you can still use them. It does require setup for transpilation so you have to use Babel or TypeScript.

### TypeScript

Enable the compiler option `"experimentalDecorators": true` and `"useDefineForClassFields": true` in your `tsconfig.json`.
Expand All @@ -89,7 +145,7 @@ Install support for decorators: `npm i --save-dev @babel/plugin-proposal-class-p

Decorators are only supported out of the box when using TypeScript in `create-react-app@^2.1.1` and newer. In older versions or when using vanilla JavaScript use eject, or the [customize-cra](https://github.com/arackaf/customize-cra) package.

## Disclaimer: Limitations of the decorator syntax
### Disclaimer: Limitations of the legacy decorator syntax

The current transpiler implementations of the decorator syntax are quite limited and don't behave exactly the same.
Also, many compositional patterns are currently not possible with decorators, until the stage-2 proposal has been implemented by all transpilers.
Expand Down
27 changes: 17 additions & 10 deletions jest.base.config.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,28 @@
const fs = require("fs")
const path = require("path")

const tsConfig = "tsconfig.test.json"

module.exports = function buildConfig(packageDirectory, pkgConfig) {
module.exports = function buildConfig(
packageDirectory,
pkgConfig,
tsConfig = "tsconfig.test.json"
) {
const packageName = require(`${packageDirectory}/package.json`).name
const packageTsconfig = path.resolve(packageDirectory, tsConfig)
return {
preset: "ts-jest/presets/js-with-ts",
testEnvironment: "jest-environment-jsdom-fifteen",
testEnvironment: "jsdom",
globals: {
__DEV__: true,
"ts-jest": {
tsconfig: fs.existsSync(packageTsconfig)
? packageTsconfig
: path.resolve(__dirname, tsConfig)
}
__DEV__: true
},
transform: {
"^.+\\.[jt]sx?$": [
"ts-jest",
{
tsconfig: fs.existsSync(packageTsconfig)
? packageTsconfig
: path.resolve(__dirname, tsConfig)
}
]
},
testRegex: "__tests__/.*\\.(j|t)sx?$",
coverageDirectory: "<rootDir>/coverage/",
Expand Down
2 changes: 1 addition & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const buildConfig = require("./jest.base.config")

module.exports = buildConfig(__dirname, {
projects: ["<rootDir>/packages/*/jest.config.js"]
projects: ["<rootDir>/packages/*/jest.config.js", "<rootDir>/packages/*/jest.config-*.js"]
// collectCoverageFrom: ["<rootDir>/packages/*/src/**/*.{ts,tsx}"]
})
17 changes: 9 additions & 8 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
"packages/*"
],
"resolutions": {
"typescript": "^4.0.2",
"jest": "^29.5.0",
"typescript": "^5.0.2",
"recast": "^0.23.1"
},
"repository": {
Expand Down Expand Up @@ -40,34 +41,34 @@
"@types/prop-types": "^15.5.2",
"@types/react": "^18.0.0",
"@types/react-dom": "^18.0.0",
"@typescript-eslint/eslint-plugin": "^4.6.1",
"@typescript-eslint/parser": "^4.1.1",
"@typescript-eslint/eslint-plugin": "^5.0.0",
"@typescript-eslint/parser": "^5.0.0",
"coveralls": "^3.1.0",
"eslint": "^6.8.0",
"execa": "^4.1.0",
"fs-extra": "9.0.1",
"husky": "^4.2.5",
"import-size": "^1.0.2",
"iterall": "^1.3.0",
"jest": "^26.6.2",
"jest-environment-jsdom-fifteen": "^1.0.2",
"jest": "^29.5.0",
"jest-environment-jsdom": "^29.5.0",
"jest-mock-console": "^1.0.1",
"lerna": "^3.22.1",
"lint-staged": "^10.1.7",
"lodash": "^4.17.4",
"minimist": "^1.2.5",
"mkdirp": "1.0.4",
"prettier": "^2.0.5",
"prettier": "^2.8.4",
"pretty-quick": "3.1.0",
"prop-types": "15.6.2",
"react": "^18.0.0",
"react-dom": "^18.0.0",
"react-test-renderer": "^18.0.0",
"serializr": "^2.0.3",
"tape": "^5.0.1",
"ts-jest": "26.4.1",
"ts-jest": "^29.0.5",
"tsdx": "^0.14.1",
"typescript": "^4.0.2"
"typescript": "^5.0.2"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is little hastle, it could be interesting to split out the tooling & infra updates (Jest and TS) from the actual changes as separate PR, which we can land quickly, given that space wise the largest updates is coming from Jest, it might be nice to have that hitched down before landing decorators, to be able to more easily roll back if needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof. That would have been a good thought for me to think of... I'll see what I can do.

},
"husky": {
"hooks": {
Expand Down
4 changes: 2 additions & 2 deletions packages/eslint-plugin-mobx/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@
"eslint": "^3.0.0 || ^4.0.0 || ^5.0.0 || ^6.0.0 || ^7.0.0 || ^8.0.0"
},
"devDependencies": {
"@typescript-eslint/eslint-plugin": "^4.29.3",
"@typescript-eslint/parser": "^4.0.0",
"@typescript-eslint/eslint-plugin": "^5.0.0",
"@typescript-eslint/parser": "^5.0.0",
"eslint": "^7.0.0",
"@babel/core": "^7.16.0",
"@babel/preset-env": "^7.16.4",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,13 @@ exports[`issue 12 run transaction 2`] = `

exports[`issue 309 isObserverBatched is still defined and yields true by default 1`] = `
[MockFunction] {
"calls": Array [
Array [
"calls": [
[
"[MobX] Deprecated",
],
],
"results": Array [
Object {
"results": [
{
"type": "return",
"value": undefined,
},
Expand All @@ -82,13 +82,13 @@ exports[`issue 309 isObserverBatched is still defined and yields true by default

exports[`issue 309 isObserverBatched is still defined and yields true by default 2`] = `
[MockFunction] {
"calls": Array [
Array [
"calls": [
[
"[MobX] Deprecated",
],
],
"results": Array [
Object {
"results": [
{
"type": "return",
"value": undefined,
},
Expand All @@ -98,13 +98,13 @@ exports[`issue 309 isObserverBatched is still defined and yields true by default

exports[`observer(cmp, { forwardRef: true }) + useImperativeHandle 1`] = `
[MockFunction] {
"calls": Array [
Array [
"calls": [
[
"[mobx-react-lite] \`observer(fn, { forwardRef: true })\` is deprecated, use \`observer(React.forwardRef(fn))\`",
],
],
"results": Array [
Object {
"results": [
{
"type": "return",
"value": undefined,
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`printDebugValue 1`] = `
Object {
"dependencies": Array [
Object {
{
"dependencies": [
{
"name": "ObservableObject@1.euro",
},
Object {
"dependencies": Array [
Object {
{
"dependencies": [
{
"name": "ObservableObject@1.euro",
},
],
Expand All @@ -20,7 +20,7 @@ Object {
`;

exports[`printDebugValue 2`] = `
Object {
{
"name": "Autorun@2",
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,20 @@

exports[`base useAsObservableSource should work with <Observer> 1`] = `
[MockFunction] {
"calls": Array [
Array [
"calls": [
[
"[mobx-react-lite] 'useAsObservableSource' is deprecated, please store the values directly in an observable, for example by using 'useLocalObservable', and sync future updates using 'useEffect' when needed. See the README for examples.",
],
Array [
[
"[mobx-react-lite] 'useLocalStore' is deprecated, use 'useLocalObservable' instead.",
],
],
"results": Array [
Object {
"results": [
{
"type": "return",
"value": undefined,
},
Object {
{
"type": "return",
"value": undefined,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@

exports[`base useLocalStore should work 1`] = `
[MockFunction] {
"calls": Array [
Array [
"calls": [
[
"[mobx-react-lite] 'useLocalStore' is deprecated, use 'useLocalObservable' instead.",
],
],
"results": Array [
Object {
"results": [
{
"type": "return",
"value": undefined,
},
Expand All @@ -18,13 +18,13 @@ exports[`base useLocalStore should work 1`] = `

exports[`is used to keep observable within component body with props and useObserver 1`] = `
[MockFunction] {
"calls": Array [
Array [
"calls": [
[
"[mobx-react-lite] 'useAsObservableSource' is deprecated, please store the values directly in an observable, for example by using 'useLocalObservable', and sync future updates using 'useEffect' when needed. See the README for examples.",
],
],
"results": Array [
Object {
"results": [
{
"type": "return",
"value": undefined,
},
Expand Down
Loading
Loading