Skip to content

Commit

Permalink
useId: Use 'H' to separate main id from hook index (#23363)
Browse files Browse the repository at this point in the history
No id should be a subset of any other id. Currently, this is not true
when there are multiple hooks in the same component. We append the
hook index to the end of the id, except for the first one. So you get
this pattern.

Before this change:

- 1st hook's id: :R0:
- 2nd hook's id: :R0:1:

The first hook's id is a subset of all the other ids in the
same component.

The fix for this is to use a different character to separate the main
id from the hook index. I've chosen a captial 'H' for this because
capital letters are not part of the base 32 character set when encoding
with `toString(32)`.

After this change:

- 1st hook's id: :R0:
- 2nd hook's id: :R0H1:
  • Loading branch information
acdlite authored Feb 25, 2022
1 parent 629036a commit 5d08a24
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 9 deletions.
4 changes: 2 additions & 2 deletions packages/react-dom/src/__tests__/ReactDOMUseId-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ describe('useId', () => {
}

function normalizeTreeIdForTesting(id) {
const result = id.match(/:(R|r)(.*):(([0-9]*):)?/);
const result = id.match(/:(R|r)([a-z0-9]*)(H([0-9]*))?:/);
if (result === undefined) {
throw new Error('Invalid id format');
}
Expand Down Expand Up @@ -342,7 +342,7 @@ describe('useId', () => {
<div
id="container"
>
:R0:, :R0:1:, :R0:2:
:R0:, :R0H1:, :R0H2:
<!-- -->
</div>
`);
Expand Down
6 changes: 3 additions & 3 deletions packages/react-dom/src/server/ReactDOMServerFormatConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -242,16 +242,16 @@ export function makeId(
): string {
const idPrefix = responseState.idPrefix;

let id = ':' + idPrefix + 'R' + treeId + ':';
let id = ':' + idPrefix + 'R' + treeId;

// Unless this is the first id at this level, append a number at the end
// that represents the position of this useId hook among all the useId
// hooks for this fiber.
if (localId > 0) {
id += localId.toString(32) + ':';
id += 'H' + localId.toString(32);
}

return id;
return id + ':';
}

function encodeHTMLTextNode(text: string): string {
Expand Down
6 changes: 4 additions & 2 deletions packages/react-reconciler/src/ReactFiberHooks.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -2072,15 +2072,17 @@ function mountId(): string {
const treeId = getTreeId();

// Use a captial R prefix for server-generated ids.
id = ':' + identifierPrefix + 'R' + treeId + ':';
id = ':' + identifierPrefix + 'R' + treeId;

// Unless this is the first id at this level, append a number at the end
// that represents the position of this useId hook among all the useId
// hooks for this fiber.
const localId = localIdCounter++;
if (localId > 0) {
id += localId.toString(32) + ':';
id += 'H' + localId.toString(32);
}

id += ':';
} else {
// Use a lowercase r prefix for client-generated ids.
const globalClientId = globalClientIdCounter++;
Expand Down
6 changes: 4 additions & 2 deletions packages/react-reconciler/src/ReactFiberHooks.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -2072,15 +2072,17 @@ function mountId(): string {
const treeId = getTreeId();

// Use a captial R prefix for server-generated ids.
id = ':' + identifierPrefix + 'R' + treeId + ':';
id = ':' + identifierPrefix + 'R' + treeId;

// Unless this is the first id at this level, append a number at the end
// that represents the position of this useId hook among all the useId
// hooks for this fiber.
const localId = localIdCounter++;
if (localId > 0) {
id += localId.toString(32) + ':';
id += 'H' + localId.toString(32);
}

id += ':';
} else {
// Use a lowercase r prefix for client-generated ids.
const globalClientId = globalClientIdCounter++;
Expand Down

0 comments on commit 5d08a24

Please sign in to comment.