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

[Fizz] Track Key Path #27243

Merged
merged 1 commit into from
Aug 18, 2023
Merged

[Fizz] Track Key Path #27243

merged 1 commit into from
Aug 18, 2023

Conversation

sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented Aug 18, 2023

Tracks the currently executing parent path of a task, using the name of the component and the key or index.

This can be used to uniquely identify an instance of a component between requests - assuming nothing in the parents has changed. Even if it has changed, if things are properly keyed, it should still line up.

It's not used yet but we'll need this for two separate features so should land this so we can stack on top.

Can be passed to JSON.stringify(...) to generate a unique key.

Tracks the currently executing parent path of a task, using the name of the
component and the key or index.

This can be used to uniquely identify and instance of a component between
requests - assuming nothing has changed.
@sebmarkbage sebmarkbage requested a review from acdlite August 18, 2023 02:38
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Aug 18, 2023
const props = element.props;
const ref = element.ref;
const name = getComponentNameFromType(type);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since we're going to go check the type again in renderElement, we could move this code deeper and extract the name in each slot but there's so much code duplication I stayed away from that even though that's my usual M.O.

@react-sizebot
Copy link

Comparing: 5623f2a...6aea9f9

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 165.67 kB 165.67 kB = 51.99 kB 51.99 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 173.16 kB 173.16 kB = 54.21 kB 54.21 kB
facebook-www/ReactDOM-prod.classic.js = 570.50 kB 570.50 kB = 100.49 kB 100.49 kB
facebook-www/ReactDOM-prod.modern.js = 554.30 kB 554.30 kB = 97.66 kB 97.66 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
facebook-www/ReactDOMServer-prod.modern.js +1.92% 145.46 kB 148.25 kB +2.22% 26.41 kB 26.99 kB
facebook-www/ReactDOMServerStreaming-prod.modern.js +1.86% 152.08 kB 154.91 kB +2.10% 28.10 kB 28.69 kB
oss-stable-semver/react-server/cjs/react-server.production.min.js +1.27% 25.08 kB 25.40 kB +1.30% 8.54 kB 8.65 kB
oss-stable/react-server/cjs/react-server.production.min.js +1.27% 25.08 kB 25.40 kB +1.30% 8.54 kB 8.65 kB
oss-experimental/react-server/cjs/react-server.production.min.js +1.24% 25.71 kB 26.03 kB +1.12% 8.77 kB 8.86 kB
oss-stable-semver/react-server/cjs/react-server.development.js +0.82% 146.93 kB 148.13 kB +0.80% 36.62 kB 36.92 kB
oss-stable/react-server/cjs/react-server.development.js +0.82% 146.93 kB 148.13 kB +0.80% 36.62 kB 36.92 kB
oss-experimental/react-server/cjs/react-server.development.js +0.81% 148.91 kB 150.11 kB +0.83% 37.04 kB 37.35 kB
facebook-www/ReactDOMServer-prod.classic.js +0.53% 148.20 kB 148.99 kB +0.64% 27.06 kB 27.23 kB
oss-stable-semver/react-dom/cjs/react-dom-server-legacy.browser.production.min.js +0.53% 60.64 kB 60.96 kB +0.51% 18.09 kB 18.18 kB
oss-stable/react-dom/cjs/react-dom-server-legacy.browser.production.min.js +0.53% 60.67 kB 60.99 kB +0.51% 18.11 kB 18.21 kB
oss-stable-semver/react-dom/umd/react-dom-server-legacy.browser.production.min.js +0.53% 60.80 kB 61.12 kB +0.55% 18.43 kB 18.54 kB
oss-stable/react-dom/umd/react-dom-server-legacy.browser.production.min.js +0.53% 60.83 kB 61.15 kB +0.55% 18.46 kB 18.56 kB
oss-stable-semver/react-dom/cjs/react-dom-server.browser.production.min.js +0.51% 62.81 kB 63.13 kB +0.62% 19.41 kB 19.53 kB
oss-stable/react-dom/cjs/react-dom-server.browser.production.min.js +0.51% 62.84 kB 63.16 kB +0.62% 19.43 kB 19.56 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.browser.production.min.js +0.51% 62.69 kB 63.01 kB +0.50% 18.88 kB 18.98 kB
oss-stable-semver/react-dom/umd/react-dom-server.browser.production.min.js +0.51% 62.96 kB 63.28 kB +0.50% 19.69 kB 19.78 kB
oss-stable/react-dom/umd/react-dom-server.browser.production.min.js +0.51% 62.99 kB 63.31 kB +0.50% 19.71 kB 19.81 kB
oss-experimental/react-dom/umd/react-dom-server-legacy.browser.production.min.js +0.51% 62.84 kB 63.16 kB +0.68% 19.26 kB 19.40 kB
oss-experimental/react-dom/cjs/react-dom-static.browser.production.min.js +0.50% 64.18 kB 64.50 kB +0.50% 19.93 kB 20.03 kB
oss-experimental/react-dom/cjs/react-dom-server.browser.production.min.js +0.50% 64.30 kB 64.62 kB +0.54% 19.97 kB 20.08 kB
oss-experimental/react-dom/umd/react-dom-server.browser.production.min.js +0.49% 64.45 kB 64.77 kB +0.35% 20.24 kB 20.31 kB
oss-experimental/react-dom/cjs/react-dom-static.edge.production.min.js +0.49% 64.51 kB 64.83 kB +0.44% 20.04 kB 20.13 kB
oss-stable-semver/react-dom/cjs/react-dom-server.bun.production.min.js +0.49% 65.18 kB 65.50 kB +0.51% 19.83 kB 19.93 kB
oss-stable/react-dom/cjs/react-dom-server.bun.production.min.js +0.49% 65.20 kB 65.52 kB +0.51% 19.86 kB 19.96 kB
oss-stable-semver/react-dom/cjs/react-dom-server-legacy.node.production.min.js +0.49% 65.42 kB 65.74 kB +0.45% 19.68 kB 19.76 kB
oss-stable/react-dom/cjs/react-dom-server-legacy.node.production.min.js +0.49% 65.45 kB 65.77 kB +0.45% 19.70 kB 19.79 kB
oss-stable-semver/react-dom/cjs/react-dom-server.edge.production.min.js +0.48% 67.04 kB 67.36 kB +0.57% 20.84 kB 20.96 kB
oss-stable/react-dom/cjs/react-dom-server.edge.production.min.js +0.48% 67.07 kB 67.39 kB +0.58% 20.87 kB 20.99 kB
oss-experimental/react-dom/cjs/react-dom-server.bun.production.min.js +0.48% 67.33 kB 67.65 kB +0.46% 20.67 kB 20.77 kB
oss-stable-semver/react-dom/cjs/react-dom-server.node.production.min.js +0.48% 67.14 kB 67.46 kB +0.48% 20.86 kB 20.96 kB
oss-stable/react-dom/cjs/react-dom-server.node.production.min.js +0.47% 67.17 kB 67.49 kB +0.48% 20.89 kB 20.99 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.node.production.min.js +0.47% 67.58 kB 67.90 kB +0.48% 20.51 kB 20.61 kB
oss-experimental/react-dom/cjs/react-dom-server.edge.production.min.js +0.46% 68.64 kB 68.96 kB +0.50% 21.45 kB 21.56 kB
oss-experimental/react-dom/cjs/react-dom-static.node.production.min.js +0.46% 68.72 kB 69.03 kB +0.45% 21.49 kB 21.59 kB
oss-experimental/react-dom/cjs/react-dom-server.node.production.min.js +0.46% 68.74 kB 69.06 kB +0.43% 21.46 kB 21.55 kB
facebook-www/ReactDOMServerStreaming-dev.modern.js +0.37% 346.97 kB 348.25 kB +0.40% 76.71 kB 77.02 kB
facebook-www/ReactDOMServer-dev.modern.js +0.36% 352.16 kB 353.44 kB +0.39% 77.95 kB 78.26 kB
facebook-www/ReactDOMServer-dev.classic.js +0.36% 359.59 kB 360.87 kB +0.39% 79.60 kB 79.91 kB
oss-stable-semver/react-dom/cjs/react-dom-server.bun.development.js +0.35% 341.13 kB 342.33 kB +0.39% 76.59 kB 76.89 kB
oss-stable/react-dom/cjs/react-dom-server.bun.development.js +0.35% 341.16 kB 342.36 kB +0.39% 76.62 kB 76.91 kB
oss-stable-semver/react-dom/cjs/react-dom-server.browser.development.js +0.35% 343.91 kB 345.11 kB +0.38% 77.49 kB 77.79 kB
oss-stable/react-dom/cjs/react-dom-server.browser.development.js +0.35% 343.93 kB 345.13 kB +0.38% 77.52 kB 77.82 kB
oss-stable-semver/react-dom/cjs/react-dom-server-legacy.browser.development.js +0.35% 344.12 kB 345.32 kB +0.38% 77.17 kB 77.47 kB
oss-stable/react-dom/cjs/react-dom-server-legacy.browser.development.js +0.35% 344.15 kB 345.35 kB +0.38% 77.20 kB 77.49 kB
oss-stable-semver/react-dom/cjs/react-dom-server.edge.development.js +0.35% 344.32 kB 345.52 kB +0.38% 77.61 kB 77.91 kB
oss-stable/react-dom/cjs/react-dom-server.edge.development.js +0.35% 344.34 kB 345.54 kB +0.38% 77.64 kB 77.93 kB
oss-stable-semver/react-dom/cjs/react-dom-server.node.development.js +0.35% 345.39 kB 346.58 kB +0.38% 77.53 kB 77.83 kB
oss-stable/react-dom/cjs/react-dom-server.node.development.js +0.35% 345.41 kB 346.61 kB +0.38% 77.56 kB 77.86 kB
oss-stable-semver/react-dom/cjs/react-dom-server-legacy.node.development.js +0.35% 345.95 kB 347.15 kB +0.38% 77.63 kB 77.93 kB
oss-stable/react-dom/cjs/react-dom-server-legacy.node.development.js +0.35% 345.98 kB 347.18 kB +0.38% 77.66 kB 77.95 kB
oss-stable-semver/react-dom/umd/react-dom-server.browser.development.js +0.34% 360.50 kB 361.73 kB +0.38% 78.41 kB 78.71 kB
oss-stable/react-dom/umd/react-dom-server.browser.development.js +0.34% 360.52 kB 361.76 kB +0.39% 78.44 kB 78.74 kB
oss-stable-semver/react-dom/umd/react-dom-server-legacy.browser.development.js +0.34% 360.73 kB 361.97 kB +0.39% 78.05 kB 78.35 kB
oss-stable/react-dom/umd/react-dom-server-legacy.browser.development.js +0.34% 360.76 kB 361.99 kB +0.39% 78.08 kB 78.38 kB
oss-experimental/react-dom/cjs/react-dom-server.bun.development.js +0.34% 351.71 kB 352.91 kB +0.39% 78.98 kB 79.28 kB
oss-experimental/react-dom/cjs/react-dom-static.browser.development.js +0.34% 353.21 kB 354.41 kB +0.38% 79.47 kB 79.78 kB
oss-experimental/react-dom/cjs/react-dom-static.edge.development.js +0.34% 353.62 kB 354.82 kB +0.39% 79.60 kB 79.91 kB
oss-experimental/react-dom/cjs/react-dom-server.browser.development.js +0.34% 353.91 kB 355.11 kB +0.38% 79.65 kB 79.96 kB
oss-experimental/react-dom/cjs/react-dom-server.edge.development.js +0.34% 354.32 kB 355.52 kB +0.39% 79.78 kB 80.09 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.browser.development.js +0.34% 354.70 kB 355.90 kB +0.38% 79.56 kB 79.86 kB
oss-experimental/react-dom/cjs/react-dom-static.node.development.js +0.34% 355.35 kB 356.55 kB +0.38% 79.80 kB 80.10 kB
oss-experimental/react-dom/cjs/react-dom-server.node.development.js +0.34% 355.39 kB 356.59 kB +0.38% 79.70 kB 80.01 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.node.development.js +0.34% 356.53 kB 357.73 kB +0.38% 80.01 kB 80.32 kB
oss-experimental/react-dom/umd/react-dom-server.browser.development.js +0.33% 370.94 kB 372.18 kB +0.35% 80.55 kB 80.84 kB
oss-experimental/react-dom/umd/react-dom-server-legacy.browser.development.js +0.33% 371.76 kB 372.99 kB +0.35% 80.43 kB 80.72 kB

Generated by 🚫 dangerJS against 6aea9f9

if (isArray(node) || getIteratorFn(node)) {
// Nested arrays behave like a "fragment node" which is keyed.
// Therefore we need to add the current index as a parent key.
task.keyPath = [task.keyPath, '', childIndex];
Copy link
Collaborator Author

@sebmarkbage sebmarkbage Aug 18, 2023

Choose a reason for hiding this comment

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

This is subtle but the first array inside of a component is the same as having a single element with slot 0. So the array itself doesn't take up slot 0. In other words, we the outer most array doesn't get added to the path but the next one gets treated as if it was a fragment element in that slot.

@sebmarkbage
Copy link
Collaborator Author

sebmarkbage commented Aug 18, 2023

This will allocate quite a lot of nodes but hopefully the young generation collector takes them quick. A possible alternative implementation could push/pop to an array in the recursive path and clone whenever it spawns a new task. We don't do that for contexts but it might make more sense for this one.

@sebmarkbage sebmarkbage merged commit 98f3f14 into facebook:main Aug 18, 2023
github-actions bot pushed a commit that referenced this pull request Aug 18, 2023
Tracks the currently executing parent path of a task, using the name of
the component and the key or index.

This can be used to uniquely identify an instance of a component between
requests - assuming nothing in the parents has changed. Even if it has
changed, if things are properly keyed, it should still line up.

It's not used yet but we'll need this for two separate features so
should land this so we can stack on top.

Can be passed to `JSON.stringify(...)` to generate a unique key.

DiffTrain build for [98f3f14](98f3f14)
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
Tracks the currently executing parent path of a task, using the name of
the component and the key or index.

This can be used to uniquely identify an instance of a component between
requests - assuming nothing in the parents has changed. Even if it has
changed, if things are properly keyed, it should still line up.

It's not used yet but we'll need this for two separate features so
should land this so we can stack on top.

Can be passed to `JSON.stringify(...)` to generate a unique key.
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
Tracks the currently executing parent path of a task, using the name of
the component and the key or index.

This can be used to uniquely identify an instance of a component between
requests - assuming nothing in the parents has changed. Even if it has
changed, if things are properly keyed, it should still line up.

It's not used yet but we'll need this for two separate features so
should land this so we can stack on top.

Can be passed to `JSON.stringify(...)` to generate a unique key.

DiffTrain build for commit 98f3f14.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants