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

Advance RFC #0566 "@cached" to Stage Recommended #914

Merged
merged 3 commits into from
Oct 6, 2023
Merged

Conversation

emberjs-rfcs-bot
Copy link
Collaborator

@emberjs-rfcs-bot emberjs-rfcs-bot commented Mar 13, 2023

Advance #566 to the Recommended Stage

Summary

This pull request is advancing the RFC to the Recommended Stage.

An FCP is required before merging this PR to advance.

Recommended Stage Summary

The "Recommended" stage is the final milestone for an RFC. It provides a signal to the wider community to indicate that a feature has been put through its ecosystem paces and is ready to use.

To reach the "Recommended" stage, the following should be true:

If appropriate, the feature is integrated into the tutorial and the guides prose. API documentation is polished and updates are carried through to other areas of API docs that may not directly pertain to the feature.

If the proposal replaces an existing feature, the addon ecosystem has largely updated to work with both old and new features.

If the proposal updates or replaces an existing feature, high-quality codemods are available.

If needed, Ember debugging tools as well as popular IDE support have been updated to support the feature.

If the feature is part of a suite of features that were designed to work together for best ergonomics, the other features are also ready to be "Recommended".

Any criteria for "Recommended" for this proposal that were established in the Ready For Release stage have been met.

An FCP is required to enter this stage. Multiple RFCs may be moved as a batch into "Recommended" with the same PR.

Checklist to move to Recommended

  • Any criteria for "Recommended" for this proposal that were established in the Ready For Release stage have been met
  • If appropriate, the feature is integrated into the tutorial and the guides prose. API documentation is polished and updates are carried through to other areas of API docs that may not directly pertain to the feature.
  • If the proposal replaces an existing feature, the addon ecosystem has largely updated to work with both old and new features.
  • If the proposal updates or replaces an existing feature, high-quality codemods are available
  • If needed, Ember debugging tools as well as popular IDE support have been updated to support the feature.
  • If the feature is part of a suite of features that were designed to work together for best ergonomics, the other features are also ready to be "Recommended".
  • This PR has been converted from a draft to a regular PR and the Final Comment Period label has been added to start the FCP

Criteria for moving to Recommended (required)

  • Fully document in Guides
  • Skim guides for places that might have related outdated information
  • Review tutorial for appropriate places to use it
  • Release new @glimmer/tracking npm package for correct types (@kategengler, @wycats) confirmed ember ships with correct types now.

@emberjs-rfcs-bot emberjs-rfcs-bot added RFC Advancement S-Recommended PR to move to the Recommended Stage labels Mar 13, 2023
@@ -8,6 +8,7 @@ teams:
- framework
prs:
accepted: 'https://github.com/emberjs/rfcs/pull/566'
recommended: 'https://github.com/emberjs/rfcs/pull/914'
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

🎉

@NullVoxPopuli
Copy link
Sponsor Contributor

NullVoxPopuli commented Mar 13, 2023

will we required to not have to have ember-cached-decorator-polyfill for providing types before we can advance this to "recommended"?

@glimmer/tracking does have @cached: https://github.com/glimmerjs/glimmer.js/blob/master/packages/%40glimmer/tracking/src/cached.ts
but it's the v2-beta -- (not present in 1.1.2).

Idk what all is involved with finalizing the v2 of @glimmer/tracking, but if I can help in anyway, lemme know

@wagenet
Copy link
Member

wagenet commented May 5, 2023

This is what I had to do to get types working:

import '@glimmer/tracking';

// We need this until we're on @glimmer/tracking 2.0 since this method only actually
// exists in Ember's special version.
declare module '@glimmer/tracking' {
	/**
	 * @decorator
	 *
	 * Memoizes the result of a getter based on autotracking.
	 *
	 * The `@cached` decorator can be used on native getters to memoize their return
	 * values based on the tracked state they consume while being calculated.
	 *
	 * By default a getter is always re-computed every time it is accessed. On
	 * average this is faster than caching every getter result by default.
	 *
	 * However, there are absolutely cases where getters are expensive, and their
	 * values are used repeatedly, so memoization would be very helpful.
	 * Strategic, opt-in memoization is a useful tool that helps developers
	 * optimize their apps when relevant, without adding extra overhead unless
	 * necessary.
	 *
	 * @example
	 *
	 * ```ts
	 * import { tracked, cached } from '@glimmer/tracking';
	 *
	 * class Person {
	 *   @tracked firstName = 'Jen';
	 *   @tracked lastName = 'Weber';
	 *
	 *   @cached
	 *   get fullName() {
	 *     return `${this.firstName} ${this.lastName}`;
	 *   }
	 * }
	 * ```
	 */
	export let cached: PropertyDecorator;
}

@wagenet
Copy link
Member

wagenet commented Aug 11, 2023

@mixonic is going to check out the docs here.

@ef4
Copy link
Contributor

ef4 commented Sep 8, 2023

The stable types in ember-source might include this without changes to @glimmer/tracking, @NullVoxPopuli is investigating.

That may or may not resolve the types question, and @mixonic is still planning to work on docs.

@NullVoxPopuli
Copy link
Sponsor Contributor

NullVoxPopuli commented Sep 8, 2023

@cached has "a" type here: https://github.com/NullVoxPopuli/cached-status-2023-09-08/blob/main/app/app.ts#L12

but is without JSDoc.
When doing "go-to-definition", you get

declare module '@ember/-internals/metal/lib/cached' {
  export const cached: PropertyDecorator;
}

which is a little goofy.
But at least it's not an error

It's also the same strategy that @tracked uses to provide types.

declare module '@ember/-internals/metal/lib/tracked' {
 // ...
}

so, I think I can PR copy-pasta some JS Doc to @cached ez
emberjs/ember.js#20543

@ef4
Copy link
Contributor

ef4 commented Sep 29, 2023

API docs are solid now.
@cached is findable in the guides via search.
built-in ember types are updated.

@ef4 ef4 marked this pull request as ready for review September 29, 2023 18:24
@achambers achambers merged commit 20bda62 into master Oct 6, 2023
8 checks passed
@delete-merged-branch delete-merged-branch bot deleted the advance-rfc-0566 branch October 6, 2023 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants