Skip to content

[Flight] Make it more obvious what the short name in the I/O description represents #33944

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jul 21, 2025
Merged

[Flight] Make it more obvious what the short name in the I/O description represents #33944

merged 2 commits into from
Jul 21, 2025
+387 −277

Conversation

eps1lon
Copy link
Collaborator

@eps1lon eps1lon commented Jul 18, 2025

This hopefully makes it more obvious what fetch (todos) means.

// URLs where we took the last path segment
-fetch (todos)
+fetch (…/todos)
// URLs with a long path segment
-fetch
+fetch (…/75897c2dcd…13d22b4b16b4)

@meta-cla meta-cla bot added the CLA Signed label Jul 18, 2025
@github-actions github-actions bot added the React Core Team Opened by a member of the React Core Team label Jul 18, 2025
@react-sizebot
Copy link

react-sizebot commented Jul 18, 2025 •
edited
Loading

Comparing: 0dca9c2...6b10c6a

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.js = 6.68 kB 6.68 kB +0.05% 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 530.70 kB 530.70 kB = 93.70 kB 93.70 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB +0.05% 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 655.25 kB 655.25 kB = 115.40 kB 115.40 kB
facebook-www/ReactDOM-prod.classic.js = 675.13 kB 675.13 kB = 118.75 kB 118.75 kB
facebook-www/ReactDOM-prod.modern.js = 665.56 kB 665.56 kB = 117.11 kB 117.11 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-experimental/react-client/cjs/react-client-flight.development.js +0.25% 159.53 kB 159.92 kB +0.30% 27.89 kB 27.97 kB
oss-experimental/react-server-dom-esm/cjs/react-server-dom-esm-client.browser.development.js +0.24% 163.62 kB 164.01 kB +0.29% 28.93 kB 29.01 kB
oss-experimental/react-server-dom-parcel/cjs/react-server-dom-parcel-client.edge.development.js +0.24% 163.63 kB 164.03 kB +0.27% 29.22 kB 29.30 kB
oss-experimental/react-server-dom-parcel/cjs/react-server-dom-parcel-client.browser.development.js +0.24% 164.76 kB 165.16 kB +0.28% 28.95 kB 29.03 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.browser.development.js +0.24% 165.77 kB 166.16 kB +0.27% 29.33 kB 29.41 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.development.js +0.24% 166.36 kB 166.76 kB +0.30% 29.47 kB 29.55 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.edge.development.js +0.24% 166.64 kB 167.03 kB +0.27% 29.69 kB 29.77 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-client.edge.development.js +0.24% 166.76 kB 167.16 kB +0.27% 29.72 kB 29.80 kB
oss-experimental/react-server-dom-esm/cjs/react-server-dom-esm-client.node.development.js +0.24% 166.84 kB 167.24 kB +0.28% 29.47 kB 29.55 kB
oss-experimental/react-server-dom-parcel/cjs/react-server-dom-parcel-client.node.development.js +0.23% 168.96 kB 169.36 kB +0.27% 29.90 kB 29.98 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-client.node.unbundled.development.js +0.23% 170.62 kB 171.01 kB +0.26% 30.19 kB 30.26 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-client.node.development.js +0.23% 171.82 kB 172.22 kB +0.27% 30.40 kB 30.48 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-client.node.development.js +0.23% 171.95 kB 172.34 kB +0.28% 30.43 kB 30.52 kB

Generated by 🚫 dangerJS against 6b10c6a

@eps1lon eps1lon force-pushed the sebbie/url-ellipsis branch 2 times, most recently from 65efc84 to d2f1717 Compare July 18, 2025 17:15
@eps1lon eps1lon requested a review from sebmarkbage July 18, 2025 17:26
@eps1lon eps1lon marked this pull request as ready for review July 18, 2025 17:26
sebmarkbage
sebmarkbage reviewed Jul 18, 2025
View reviewed changes
packages/react-client/src/ReactFlightPerformanceTrack.js
// Include the slash to make it more obvious what we trimmed.
desc = ' (…' + description.slice(slashIdx, queryIdx) + ')';
} else {
// cut out the middle to not exceed the max length
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure this is better. The max length is assuming that most things will be either below the limit or excluded all together. This makes it so that much more is always the exact limit which is large. Making it much noisier.

Additionally, Chrome will insert its own ellipsis in the middle when it's collapsed which can lead to two.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would be better to just elide it if it's long or increase the limit and let Chrome insert the ellipsis in the middle.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Additionally, Chrome will insert its own ellipsis in the middle when it's collapsed which can lead to two.

I guess the original idea was that we don't want to make the track too busy so a limit does make sense. The double ellipsis is unfortunate. Eliding the value felt pretty bad. It looked like some information was missing.

@eps1lon eps1lon force-pushed the sebbie/url-ellipsis branch from d2f1717 to 6b10c6a Compare July 21, 2025 17:42
@eps1lon
Copy link
Collaborator Author

eps1lon commented Jul 21, 2025

Going to experiment if that feels better. Eliding the description on long values was confusing.

@eps1lon eps1lon merged commit ac7da9d into facebook:main Jul 21, 2025
242 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sebmarkbage sebmarkbage sebmarkbage approved these changes

Assignees
No one assigned
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@eps1lon @react-sizebot @sebmarkbage