Advance RFC #0659 "unique-id helper" to Stage Recommended

[emberjs-rfcs-bot]

Advance #659 to the Recommended Stage

Summary

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

• PR to Accepted Stage: unique-id helper #659
• PR to Ready For Release Stage — Missing, since this preceeds the Stages RFC
• PR to Released Stage — Missing, since this preceeds the Stages RFC

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)

Since this was merged prior to the Stages RFC there were no prior criteria. However, the following have been identified as issues that should be been resolved earlier in the process:

• Merge Create public import for uniqueId helper ember.js#20171 or [FEATURE] Create public import for uniqueId helper #20171  ember.js#20464
• Remove canary feature flag Remove flags for released features ember.js#20345
• Document existing SSR caveats (no longer relevant)
• Add to guides Demonstrating unique-id helper ember-learn/guides-source#1981

[chriskrycho]

👍🏼 for TS (and, the bot will say, for Framework also; if another Framework member wants to also 👍🏼 that seems good).

[kategengler]

@chriskrycho Approving should probably wait until the PR is out of draft.

Since this is our first Advance to Recommended PR it may be worth reviewing the definition of Recommended (available in the top PR comment as "Recommended Stage Summary).

[chriskrycho]

Thanks for the clarification and helping me (and the rest of us!) through this one, @kategengler! 😓

I'll also note, after talking through this with Katie in DM, that I don't think this is technically even Ready for Release, much less Released or Recommended, because we should not call it "shipped" until emberjs/ember.js#20171 is done.

[wagenet]

@chriskrycho this is already released and people are using it. However, we should have done a better job in some of the areas before releasing. I've added those to the criteria here.

[chriskrycho]

I think there’s an interesting process question here:

This is already released and people are using it.

My understanding of the “Ready for Release” phase is not that some parts of the feature are released but that the whole thing is. Is that incorrect? (“Released” is thus different from “Parts of the actual code are in a release” in my understanding… which could be wrong!)

[bertdeblock]

I think it would also be a good idea to update https://guides.emberjs.com/release/components/built-in-components/#toc_ways-to-associate-labels-and-inputs to use the unique-id helper. Linking labels to inputs was one of the reasons for introducing the unique-id helper.

[wagenet]

@chriskrycho the description of Ready for Release says:

At this stage, features or deprecations may be available for use behind a feature flag, or with an optional package, etc.

However, while the flag still exists I believe it has now been set to true by default, so it's not really behind the flag.

[chriskrycho]

Right, but what I'm getting at is that part of the feature isn't even implemented yet! It's a thing we identified after the feature flag was flipped off because of how it interacts with other features also landing lately. One move might be to say that we fix this issue as part of those; but at a minimum it's a blocker for Recommended and it's the kind of thing we should think about in terms of updating recent RFC'd features.

[wagenet]

@chriskrycho I agree that we might have wanted to stop this at a sooner stage if we were doing it over again.

[MelSumner]

@mansona can look at this after EmberConf 2023. Mel will follow-up. Ed is willing to spend some time pairing to move this forward.

[MelSumner]

Also: let's make sure that the SSR caveat is documented in the API docs so folks who don't need SSR can use it freely.

[NullVoxPopuli]

What is the SSR caveat? this is a helper, so it doesn't have the same constraints as modifiers (which are known to not work in SSR)
am I missing something?

[ef4]

What is the SSR caveat? this is a helper, so it doesn't have the same constraints as modifiers (which are known to not work in SSR) am I missing something?

I think the issue is that the unique-id is not stable across rehydration.

[ef4]

This RFC is waiting on someone to volunteer to document the SSR issues.

[wagenet]

@mansona is hoping to be able to do the documentation work after consulting with @wycats.

[ef4]

This may need another volunteer to finish the docs.

[ef4]

The remaining task here is to document SSR caveats, but it's unclear to me what that is.

I understand that the value of the id can change at rehydration, but why does that matter? If it changes consistently in all the places that use it, that shouldn't matter?

[NullVoxPopuli]

@wycats,

We don't have the SSR issues that React has had, because React's ecosystem tends to, by default, store mutable state in module-scope, and folks couldn't agree on an implementation of unique ids, so they had an issue where folks would have duplicate ids (a few reasons: due to multiple implementations using let vars in modulespace, or multiple apps being rendered on the screen at the same time (and for SSR, the way react did server-side component rendering _every page was a duplicate app, thus re-using IDs))

• Generating random/unique attributes server-side that don't break client-side mounting facebook/react#4000
• Unique component identifier facebook/react#1137
• Generating unique ID's and SSR (for a11y and more) facebook/react#5867
• [Flight] Implement useId hook facebook/react#24172

To "fix", their useId hook adds a prefix based on the request:

const id = currentRequest.identifierCount++;
// use 'S' for Flight components to distinguish from 'R' and 'r' in Fizz/Client
return ':' + currentRequest.identifierPrefix + 'S' + id.toString(32) + ':';

where

    identifierPrefix: identifierPrefix || '',
    identifierCount: 1,

Our Unique Id implementation uses "randomness", so it's unlikely we'll have duplicates.

  return ([3e7] + -1e3 + -4e3 + -2e3 + -1e11).replace(/[0-3]/g, (a) =>
    ((a * 4) ^ ((Math.random() * 16) >> (a & 2))).toString(16)
  );

So we can be reasonably sure all usages are unique across the component / module graph as well as each render.

[ef4]

This agrees with what I was able to find too. Their issue was that they would actually get mismatches between the <input> and the <label>. We don't have any known cases that produce mismatches, even when rehydrating SSR.

[ef4]

1. We have confirmed there's no SSR caveat to document.

2. But we also still needed to add to guides. Open PR for guides: Demonstrating unique-id helper ember-learn/guides-source#1981