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

Implement identifierPrefix option for useId #22855

Merged
merged 1 commit into from
Dec 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion packages/react-art/src/ReactART.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,15 @@ class Surface extends React.Component {

this._surface = Mode.Surface(+width, +height, this._tagRef);

this._mountNode = createContainer(this._surface, LegacyRoot, false, null);
this._mountNode = createContainer(
this._surface,
LegacyRoot,
false,
null,
false,
false,
'',
);
updateContainer(this.props.children, this._mountNode, this);
}

Expand Down
64 changes: 63 additions & 1 deletion packages/react-dom/src/__tests__/ReactDOMUseId-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ describe('useId', () => {

function normalizeTreeIdForTesting(id) {
const [serverClientPrefix, base32, hookIndex] = id.split(':');
if (serverClientPrefix === 'r') {
if (serverClientPrefix.endsWith('r')) {
// Client ids aren't stable. For testing purposes, strip out the counter.
return (
'CLIENT_GENERATED_ID' +
Expand Down Expand Up @@ -569,4 +569,66 @@ describe('useId', () => {
// Should have hydrated successfully
expect(span.current).toBe(dehydratedSpan);
});

test('identifierPrefix option', async () => {
function Child() {
const id = useId();
return <div>{id}</div>;
}

function App({showMore}) {
return (
<>
<Child />
<Child />
{showMore && <Child />}
</>
);
}

await serverAct(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />, {
identifierPrefix: 'custom-prefix-',
});
pipe(writable);
});
let root;
await clientAct(async () => {
root = ReactDOM.hydrateRoot(container, <App />, {
identifierPrefix: 'custom-prefix-',
});
});
expect(container).toMatchInlineSnapshot(`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh wow we can write assertions like this??

<div
id="container"
>
<div>
custom-prefix-R:1
</div>
<div>
custom-prefix-R:2
</div>
</div>
`);

// Mount a new, client-only id
await clientAct(async () => {
root.render(<App showMore={true} />);
});
expect(container).toMatchInlineSnapshot(`
<div
id="container"
>
<div>
custom-prefix-R:1
</div>
<div>
custom-prefix-R:2
</div>
<div>
custom-prefix-r:0
</div>
</div>
`);
});
});
1 change: 1 addition & 0 deletions packages/react-dom/src/client/ReactDOMLegacy.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ function legacyCreateRootFromDOMContainer(
null, // hydrationCallbacks
false, // isStrictMode
false, // concurrentUpdatesByDefaultOverride,
'', // identiferPrefix
);
markContainerAsRoot(root.current, container);

Expand Down
53 changes: 38 additions & 15 deletions packages/react-dom/src/client/ReactDOMRoot.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export type CreateRootOptions = {
// END OF TODO
unstable_strictMode?: boolean,
unstable_concurrentUpdatesByDefault?: boolean,
identifierPrefix?: string,
...
};

Expand All @@ -43,6 +44,7 @@ export type HydrateRootOptions = {
// Options for all roots
unstable_strictMode?: boolean,
unstable_concurrentUpdatesByDefault?: boolean,
identifierPrefix?: string,
...
};

Expand Down Expand Up @@ -158,13 +160,22 @@ export function createRoot(
null;
// END TODO

const isStrictMode = options != null && options.unstable_strictMode === true;
let concurrentUpdatesByDefaultOverride = null;
if (allowConcurrentByDefault) {
concurrentUpdatesByDefaultOverride =
options != null && options.unstable_concurrentUpdatesByDefault != null
? options.unstable_concurrentUpdatesByDefault
: null;
let isStrictMode = false;
let concurrentUpdatesByDefaultOverride = false;
let identifierPrefix = '';
if (options !== null && options !== undefined) {
if (options.unstable_strictMode === true) {
isStrictMode = true;
}
if (
allowConcurrentByDefault &&
options.unstable_concurrentUpdatesByDefault === true
) {
concurrentUpdatesByDefaultOverride = true;
}
if (options.identifierPrefix !== undefined) {
identifierPrefix = options.identifierPrefix;
}
}

const root = createContainer(
Expand All @@ -174,6 +185,7 @@ export function createRoot(
hydrationCallbacks,
isStrictMode,
concurrentUpdatesByDefaultOverride,
identifierPrefix,
);
markContainerAsRoot(root.current, container);

Expand Down Expand Up @@ -217,15 +229,25 @@ export function hydrateRoot(
// For now we reuse the whole bag of options since they contain
// the hydration callbacks.
const hydrationCallbacks = options != null ? options : null;
// TODO: Delete this option
const mutableSources = (options != null && options.hydratedSources) || null;
const isStrictMode = options != null && options.unstable_strictMode === true;

let concurrentUpdatesByDefaultOverride = null;
if (allowConcurrentByDefault) {
concurrentUpdatesByDefaultOverride =
options != null && options.unstable_concurrentUpdatesByDefault != null
? options.unstable_concurrentUpdatesByDefault
: null;

let isStrictMode = false;
let concurrentUpdatesByDefaultOverride = false;
let identifierPrefix = '';
if (options !== null && options !== undefined) {
if (options.unstable_strictMode === true) {
isStrictMode = true;
}
if (
allowConcurrentByDefault &&
options.unstable_concurrentUpdatesByDefault === true
) {
concurrentUpdatesByDefaultOverride = true;
}
if (options.identifierPrefix !== undefined) {
identifierPrefix = options.identifierPrefix;
}
}

const root = createContainer(
Expand All @@ -235,6 +257,7 @@ export function hydrateRoot(
hydrationCallbacks,
isStrictMode,
concurrentUpdatesByDefaultOverride,
identifierPrefix,
);
markContainerAsRoot(root.current, container);
// This can't be a comment node since hydration doesn't work on comment nodes anyway.
Expand Down
21 changes: 21 additions & 0 deletions packages/react-dom/src/server/ReactDOMServerFormatConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export type ResponseState = {
placeholderPrefix: PrecomputedChunk,
segmentPrefix: PrecomputedChunk,
boundaryPrefix: string,
idPrefix: string,
nextSuspenseID: number,
sentCompleteSegmentFunction: boolean,
sentCompleteBoundaryFunction: boolean,
Expand Down Expand Up @@ -125,6 +126,7 @@ export function createResponseState(
placeholderPrefix: stringToPrecomputedChunk(idPrefix + 'P:'),
segmentPrefix: stringToPrecomputedChunk(idPrefix + 'S:'),
boundaryPrefix: idPrefix + 'B:',
idPrefix: idPrefix + 'R:',
nextSuspenseID: 0,
sentCompleteSegmentFunction: false,
sentCompleteBoundaryFunction: false,
Expand Down Expand Up @@ -229,6 +231,25 @@ export function assignSuspenseBoundaryID(
);
}

export function makeId(
responseState: ResponseState,
treeId: string,
localId: number,
): string {
const idPrefix = responseState.idPrefix;

let id = idPrefix + 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);
}

return id;
}

function encodeHTMLTextNode(text: string): string {
return escapeTextForBrowser(text);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export type ResponseState = {
placeholderPrefix: PrecomputedChunk,
segmentPrefix: PrecomputedChunk,
boundaryPrefix: string,
idPrefix: string,
nextSuspenseID: number,
sentCompleteSegmentFunction: boolean,
sentCompleteBoundaryFunction: boolean,
Expand All @@ -54,6 +55,7 @@ export function createResponseState(
placeholderPrefix: responseState.placeholderPrefix,
segmentPrefix: responseState.segmentPrefix,
boundaryPrefix: responseState.boundaryPrefix,
idPrefix: responseState.idPrefix,
nextSuspenseID: responseState.nextSuspenseID,
sentCompleteSegmentFunction: responseState.sentCompleteSegmentFunction,
sentCompleteBoundaryFunction: responseState.sentCompleteBoundaryFunction,
Expand All @@ -79,6 +81,7 @@ export {
getChildFormatContext,
UNINITIALIZED_SUSPENSE_BOUNDARY_ID,
assignSuspenseBoundaryID,
makeId,
pushStartInstance,
pushEndInstance,
pushStartCompletedSuspenseBoundary,
Expand Down
1 change: 1 addition & 0 deletions packages/react-native-renderer/src/ReactFabric.js
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ function render(
null,
false,
null,
'',
);
roots.set(containerTag, root);
}
Expand Down
10 changes: 9 additions & 1 deletion packages/react-native-renderer/src/ReactNativeRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,15 @@ function render(
if (!root) {
// TODO (bvaughn): If we decide to keep the wrapper component,
// We could create a wrapper for containerTag as well to reduce special casing.
root = createContainer(containerTag, LegacyRoot, false, null, false, null);
root = createContainer(
containerTag,
LegacyRoot,
false,
null,
false,
null,
'',
);
roots.set(containerTag, root);
}
updateContainer(element, root, null, callback);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,14 @@ export function assignSuspenseBoundaryID(
return responseState.nextSuspenseID++;
}

export function makeId(
responseState: ResponseState,
treeId: string,
localId: number,
): string {
throw new Error('Not implemented');
}

const RAW_TEXT = stringToPrecomputedChunk('RCTRawText');

export function pushTextInstance(
Expand Down
4 changes: 4 additions & 0 deletions packages/react-noop-renderer/src/createReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -973,6 +973,8 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
false,
null,
null,
false,
'',
);
return {
_Scheduler: Scheduler,
Expand Down Expand Up @@ -1000,6 +1002,8 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
false,
null,
null,
false,
'',
);
return {
_Scheduler: Scheduler,
Expand Down
12 changes: 10 additions & 2 deletions packages/react-reconciler/src/ReactFiberHooks.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -2035,12 +2035,20 @@ export function getIsUpdatingOpaqueValueInRenderPhaseInDEV(): boolean | void {
function mountId(): string {
const hook = mountWorkInProgressHook();

const root = ((getWorkInProgressRoot(): any): FiberRoot);
// TODO: In Fizz, id generation is specific to each server config. Maybe we
// should do this in Fiber, too? Deferring this decision for now because
// there's no other place to store the prefix except for an internal field on
// the public createRoot object, which the fiber tree does not currently have
// a reference to.
const identifierPrefix = root.identifierPrefix;

let id;
if (getIsHydrating()) {
const treeId = getTreeId();

// Use a captial R prefix for server-generated ids.
id = '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
Expand All @@ -2052,7 +2060,7 @@ function mountId(): string {
} else {
// Use a lowercase r prefix for client-generated ids.
const globalClientId = globalClientIdCounter++;
id = 'r:' + globalClientId.toString(32);
id = identifierPrefix + 'r:' + globalClientId.toString(32);
}

hook.memoizedState = id;
Expand Down
12 changes: 10 additions & 2 deletions packages/react-reconciler/src/ReactFiberHooks.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -2035,12 +2035,20 @@ export function getIsUpdatingOpaqueValueInRenderPhaseInDEV(): boolean | void {
function mountId(): string {
const hook = mountWorkInProgressHook();

const root = ((getWorkInProgressRoot(): any): FiberRoot);
// TODO: In Fizz, id generation is specific to each server config. Maybe we
// should do this in Fiber, too? Deferring this decision for now because
// there's no other place to store the prefix except for an internal field on
// the public createRoot object, which the fiber tree does not currently have
// a reference to.
const identifierPrefix = root.identifierPrefix;

let id;
if (getIsHydrating()) {
const treeId = getTreeId();

// Use a captial R prefix for server-generated ids.
id = '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
Expand All @@ -2052,7 +2060,7 @@ function mountId(): string {
} else {
// Use a lowercase r prefix for client-generated ids.
const globalClientId = globalClientIdCounter++;
id = 'r:' + globalClientId.toString(32);
id = identifierPrefix + 'r:' + globalClientId.toString(32);
}

hook.memoizedState = id;
Expand Down
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactFiberReconciler.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ export function createContainer(
hydrationCallbacks: null | SuspenseHydrationCallbacks,
isStrictMode: boolean,
concurrentUpdatesByDefaultOverride: null | boolean,
identifierPrefix: string,
): OpaqueRoot {
return createFiberRoot(
containerInfo,
Expand All @@ -249,6 +250,7 @@ export function createContainer(
hydrationCallbacks,
isStrictMode,
concurrentUpdatesByDefaultOverride,
identifierPrefix,
);
}

Expand Down
Loading