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

Stable decorators #20868

Draft
wants to merge 24 commits into
base: main
Choose a base branch
from
Draft

Stable decorators #20868

wants to merge 24 commits into from

Conversation

ef4
Copy link
Contributor

@ef4 ef4 commented Mar 9, 2025

Working on support for the stage3 decorator spec.

This branch is an attempt at complete drop-in support for all our decorators, even the wacky and wonderful old ones. I have the entire ember test suite passing locally.

My goal here is to show everything working with only the tiniest possible semantic changes and no mandatory syntactic change to auto-accessors.

You are still likely to want to convert to auto-accessors because they will perform much better. But keeping the old syntax working, even at a perf cost, would greatly ease upgrades.

Known semantic changes

  • initialization order of field decorators changes to match the normal (non-decorated) field initialization order. Previously, decorated properties could actually influence the initialization order by pulling on each other.

TODO

  • implement auto-accessor versions of tracked and service. (I've already done this on prior attempts, it's the easier part. But I'll need to segregate them in their own test suite area, because I'm trying to keep everything working under both builds, and the old build won't support the syntax.)
    • accessor tracked
    • accessor service
  • reflect the change in the types. This is kinda annoying, because TypeScript's stable decorators are not allowed to use declare. We probably can't make ember's own build typecheck under both configurations, but we can probably offer enough overrides to demonstrate different apps working under both configurations.
  • in CI, run the ember test suite with stable decorators
  • in CI, run one or more of the smoke test apps with stable decorators.
  • Question: should the accessor form of tracked get all the same computed property interop as the field form?
    • Pro argument: yes because people don't know where they're relying on the interop
    • Con argument:
      • meta.writeDescriptors can't happen at class construction time, it would need to happen via addInitializer the first time a class is instantiated. (Our field form of tracked does a bunch of that, but the messiness there is more excusable since that's the allowed-to-be-slower legacy form.) This injects a little code into every object instantiation. Even in new apps that don't actually need it.
    • I think the best answer here would require some related work on how to segregate off the computed system so that it's still available when you need it and not there when you don't.

@ef4 ef4 force-pushed the stable-field-decorators branch from 829a516 to 2dac217 Compare March 9, 2025 21:32
Rollup was actually dropping a side-effect that was intentionally part of a test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant