Skip to content

Deprecate the computed macros (RFC 1234) - #21688

Open
NullVoxPopuli-ai-agent wants to merge 2 commits into
emberjs:mainfrom
NullVoxPopuli-ai-agent:nvp/deprecate-computed-macros
Open

NullVoxPopuli-ai-agent wants to merge 2 commits into
emberjs:mainfrom
NullVoxPopuli-ai-agent:nvp/deprecate-computed-macros

Conversation

@NullVoxPopuli-ai-agent

Copy link
Copy Markdown
Contributor

Deprecates the macros of @ember/object/computed that had no deprecation yet. This is one part of RFC 1234, split from the spike in #21674 so that it can land alone.

Guide: ember-learn/deprecation-app#1440. It links to the Octane migration guide.

What is deprecated

alias, and, bool, deprecatingAlias, equal, gt, gte, lt, lte,
match, not, oneWay, or, readOnly, reads
  • Id: deprecate-computed-macros, available in 7.5.0, until 9.0.0, not enabled.
  • Message: `not` from `@ember/object/computed` is deprecated. Use a getter instead.
  • Where: each macro reports after its decorator-misuse assertion, so the assertion tests need no change.
  • alias: the public alias is now a function in @ember/object/computed. It reports and then calls the implementation in @ember/-internals/metal.

What is not in this PR

Ember code that used a macro

RouterService and the private routing service used the public readOnly. They now build the same read-only alias from the implementation in @ember/-internals/metal, so their behavior does not change.

I first tried getters with @dependentKeyCompat. That broke a classic computed property that depends on router.currentURL: a second reader of the getter can hide an invalidation from the computed property. A new test in currenturl_lifecycle_test.js covers this case.

Tests

  • Tests of the macros are skipped when the deprecation is removed, and expect the deprecation when it is enabled.
  • Tests of other features no longer use the macros. The router service test uses native getters, and one ArrayProxy test uses computed.

Local results:

Mode Result
Default 9291 tests, 0 failures
ALL_DEPRECATIONS_ENABLED=true 9291 tests, 0 failures
OVERRIDE_DEPRECATION_VERSION=15.0.0 8527 tests, 0 failures

The type check of the internals, the type tests, eslint, prettier and the docs coverage check pass. pnpm build:js causes no change to package.json.

Later work

🤖 Generated with Claude Code

Add the `deprecate-computed-macros` deprecation for the macros of
`@ember/object/computed` that had no deprecation yet: `alias`, `and`,
`bool`, `deprecatingAlias`, `equal`, `gt`, `gte`, `lt`, `lte`, `match`,
`not`, `oneWay`, `or`, `readOnly` and `reads`.

Each macro reports after its decorator-misuse assertion. The public
`alias` is now a function in `@ember/object/computed` that reports and
then calls the implementation in `@ember/-internals/metal`.

`RouterService` and the private routing service used the public
`readOnly`. They now build the same read-only alias from the
implementation, so their behavior does not change. A getter with
`@dependentKeyCompat` is not a replacement here: a second reader of the
getter can hide an invalidation from a classic computed property that
depends on it. A new test covers a computed property that depends on
`router.currentURL`.

Tests of the macros are skipped when the deprecation is removed, and
expect the deprecation when it is enabled. Tests of other features no
longer use the macros.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this whole file gets to be deleted when we get rid of array proxy, so I don't super care to move computed and observer out of here

import type RouterState from './router_state';
import { ROUTER } from '@ember/routing/router-service';

function readOnly(dependentKey: string) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sneaky

@NullVoxPopuli

Copy link
Copy Markdown
Contributor

in RFC #1234, we "left it open" to decide whether to add traditional deprecations for the things we are deprecating.

I think for EmberObject perhaps, the traditional deprecation system will be too noisy -- and we can do the "deprecate not having the flag set correctly".

The main benefit we get from the traditional deprecation system is the stack trace for where the deprecation is originating from.

The computed macros, I think, would be infrequently used enough where it still makes sense to use the deprecation system.

For ease of review, I've split this out to its own PR, so that we can discuss this change on its own, and I'm still figuring out what needs to happen in what order for EmberObject (and behavior svelting)

The test body runs only in debug builds and expects zero assertions in
production. The deprecation expectation adds an assertion, so it now
sits in the debug branch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

2 participants