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

@action field uses addInitializer from decorator context but that does not exist in the (new) spec #3817

Open
phiresky opened this issue Jan 19, 2024 · 2 comments
Labels

Comments

@phiresky
Copy link
Contributor

phiresky commented Jan 19, 2024

Intended outcome:

@action decorators should work with fields (lambdas).

Actual outcome:

It throws addInitializer is not a function. The reason is this code:

const { kind, name, addInitializer } = context
const ann = this
const _createAction = m =>
createAction(ann.options_?.name ?? name!.toString(), m, ann.options_?.autoAction ?? false)
// Backwards/Legacy behavior, expects makeObservable(this)
if (kind == "field") {
addInitializer(function () {

How to reproduce the issue:

@action foo = () => {};

The context passed by the decorator looks like this: {"kind":"field","name":"addFiles","static":false,"private":false,"metadata":{},"access":{}}
Versions

Spec reference
If you look at the class field spec, it says this: https://github.com/tc39/proposal-decorators#class-fields

Since class fields already return an initializer, they do not receive addInitializer and cannot add additional initialization logic.

The interface of the decorator looks like this:

type ClassFieldDecorator = (value: undefined, context: {
  kind: "field";
  name: string | symbol;
  access: { get(): unknown, set(value: unknown): void };
  static: boolean;
  private: boolean;
}) => (initialValue: unknown) => unknown | void;

So probably instead of using addInitializer the initializer should be returned.

cc @Matchlighter

I think this issue is not visible when using babel because babel does support addInitializer (in conflict with the spec). But it does appear when compiling with swc (which follows the spec)

@phiresky
Copy link
Contributor Author

Additional info. Looks like the spec readme text is outdated against the spec text and this might be an swc bug (see referenced issues)

@Matchlighter
Copy link
Collaborator

Matchlighter commented Jan 19, 2024

@phiresky The spec has been revised - addinitializer was added to field decorators for consistency, but, yeah, it looks like the README in the GitHub repo has not been updated to reflect (looks like it hasn't been updated for 3 years, but revisions have been made to the spec since).

I'm not super familiar with SWC, but high-level it re-implements Typescript's transpiler, yeah? Otherwise, I would think that maybe update TS, but I don't think that's applicable.

I have no authority of the repo here, so I'll let someone else weigh-in more officially, but my thought would be to look for/post an issue with SWC (if all versions there are current) and use patch-package (to tweak MobX away from addInitializer) in the meantime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants