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

[Flight] Consistently flag debugOwner field #30400

Merged
merged 1 commit into from
Jul 20, 2024

Conversation

sebmarkbage
Copy link
Collaborator

This is available outside enableOwnerStack but we weren't consistently using it and so it has different hidden classes.

Copy link

vercel bot commented Jul 19, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 19, 2024 2:13am

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Jul 19, 2024
@react-sizebot
Copy link

react-sizebot commented Jul 19, 2024

Comparing: 163365a...68a02cc

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 = 1.83 kB 1.82 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 499.44 kB 499.44 kB = 89.59 kB 89.59 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 504.26 kB 504.26 kB = 90.30 kB 90.30 kB
facebook-www/ReactDOM-prod.classic.js = 599.20 kB 599.20 kB = 105.80 kB 105.79 kB
facebook-www/ReactDOM-prod.modern.js = 573.37 kB 573.37 kB = 101.68 kB 101.68 kB
test_utils/ReactAllWarnings.js Deleted 64.07 kB 0.00 kB Deleted 15.86 kB 0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable-rc/react-server/cjs/react-server-flight.development.js +0.46% 80.05 kB 80.42 kB +0.24% 14.87 kB 14.90 kB
oss-stable-semver/react-server/cjs/react-server-flight.development.js +0.46% 80.05 kB 80.42 kB +0.24% 14.87 kB 14.90 kB
oss-stable/react-server/cjs/react-server-flight.development.js +0.46% 80.05 kB 80.42 kB +0.24% 14.87 kB 14.90 kB
oss-stable-rc/react-server-dom-esm/cjs/react-server-dom-esm-server.node.development.js +0.31% 120.03 kB 120.39 kB +0.18% 22.39 kB 22.43 kB
oss-stable-semver/react-server-dom-esm/cjs/react-server-dom-esm-server.node.development.js +0.31% 120.03 kB 120.39 kB +0.18% 22.39 kB 22.43 kB
oss-stable/react-server-dom-esm/cjs/react-server-dom-esm-server.node.development.js +0.31% 120.03 kB 120.39 kB +0.18% 22.39 kB 22.43 kB
oss-stable-rc/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.browser.development.js +0.30% 123.79 kB 124.16 kB +0.20% 22.98 kB 23.03 kB
oss-stable-semver/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.browser.development.js +0.30% 123.79 kB 124.16 kB +0.20% 22.98 kB 23.03 kB
oss-stable/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.browser.development.js +0.30% 123.79 kB 124.16 kB +0.20% 22.98 kB 23.03 kB
oss-stable-rc/react-server-dom-webpack/cjs/react-server-dom-webpack-server.browser.development.js +0.29% 124.41 kB 124.78 kB +0.20% 23.17 kB 23.21 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-server.browser.development.js +0.29% 124.41 kB 124.78 kB +0.20% 23.17 kB 23.21 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-server.browser.development.js +0.29% 124.41 kB 124.78 kB +0.20% 23.17 kB 23.21 kB
oss-stable-rc/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.edge.development.js +0.29% 124.86 kB 125.23 kB +0.16% 23.19 kB 23.23 kB
oss-stable-semver/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.edge.development.js +0.29% 124.86 kB 125.23 kB +0.16% 23.19 kB 23.23 kB
oss-stable/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.edge.development.js +0.29% 124.86 kB 125.23 kB +0.16% 23.19 kB 23.23 kB
oss-stable-rc/react-server-dom-webpack/cjs/react-server-dom-webpack-server.edge.development.js +0.29% 125.01 kB 125.38 kB +0.16% 23.26 kB 23.30 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-server.edge.development.js +0.29% 125.01 kB 125.38 kB +0.16% 23.26 kB 23.30 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-server.edge.development.js +0.29% 125.01 kB 125.38 kB +0.16% 23.26 kB 23.30 kB
oss-stable-rc/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.node.unbundled.development.js +0.29% 126.39 kB 126.76 kB +0.19% 23.34 kB 23.39 kB
oss-stable-semver/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.node.unbundled.development.js +0.29% 126.39 kB 126.76 kB +0.19% 23.34 kB 23.39 kB
oss-stable/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.node.unbundled.development.js +0.29% 126.39 kB 126.76 kB +0.19% 23.34 kB 23.39 kB
oss-stable-rc/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.unbundled.development.js +0.29% 126.55 kB 126.92 kB +0.19% 23.41 kB 23.45 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.unbundled.development.js +0.29% 126.55 kB 126.92 kB +0.19% 23.41 kB 23.45 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.unbundled.development.js +0.29% 126.55 kB 126.92 kB +0.19% 23.41 kB 23.45 kB
oss-stable-rc/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.node.development.js +0.29% 127.50 kB 127.87 kB +0.19% 23.62 kB 23.66 kB
oss-stable-semver/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.node.development.js +0.29% 127.50 kB 127.87 kB +0.19% 23.62 kB 23.66 kB
oss-stable/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.node.development.js +0.29% 127.50 kB 127.87 kB +0.19% 23.62 kB 23.66 kB
oss-stable-rc/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.development.js +0.29% 127.65 kB 128.01 kB +0.19% 23.68 kB 23.73 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.development.js +0.29% 127.65 kB 128.01 kB +0.19% 23.68 kB 23.73 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.development.js +0.29% 127.65 kB 128.01 kB +0.19% 23.68 kB 23.73 kB
test_utils/ReactAllWarnings.js Deleted 64.07 kB 0.00 kB Deleted 15.86 kB 0.00 kB

Generated by 🚫 dangerJS against 68a02cc

@sebmarkbage sebmarkbage merged commit f603426 into facebook:main Jul 20, 2024
190 checks passed
sebmarkbage added a commit that referenced this pull request Jul 22, 2024
Stacked on #30400 and
#30369

Previously we were using fake evals to recreate a stack for console
replaying and thrown errors. However, for owner stacks we just used the
raw string that came from the server.

This means that the format of the owner stack could include different
formats. Like Spidermonkey format for the client components and V8 for
the server components. This means that this stack can't be parsed
natively by the browser like when printing them as error like in
#30289. Additionally, since
there's no source file registered with that name and no source mapping
url, it can't be source mapped.

Before:

<img width="1329" alt="before-firefox"
src="https://github.com/user-attachments/assets/cbe03f9c-96ac-48fb-b58f-f3a224a774f4">

Instead, we need to create a fake stack like we do for the other things.
That way when it's printed as an Error it gets source mapped. It also
means that the format is consistently in the native format of the
current browser.

After:

<img width="753" alt="after-firefox"
src="https://github.com/user-attachments/assets/b436f1f5-ca37-4203-b29f-df9828c9fad3">

So this is nice because you can just take the result from
`captureOwnerStack()` and append it to an `Error` stack and print it
natively. E.g. this is what React DevTools will do.

If you want to parse and present it yourself though it's a bit awkward
though. The `captureOwnerStack()` API now includes a bunch of
`rsc://React/` URLs. These don't really have any direct connection to
the source map. Only the browser knows this connection from the eval.
You basically have to strip the prefix and then manually pass the
remainder to your own `findSourceMapURL`.

Another awkward part is that since Safari doesn't support eval sourceURL
exposed into `error.stack` - it means that `captureOwnerStack()` get an
empty location for server components since the fake eval doesn't work
there. That's not a big deal since these stacks are already broken even
for client modules for many because the `eval-source-map` strategy in
Webpack doesn't work in Safari for this same reason.

A lot of this refactoring is just clarifying that there's three kind of
ReactComponentInfo fields:

- `stack` - The raw stack as described on the original server.
- `debugStack` - The Error object containing the stack as represented in
the current client as fake evals.
- `debugTask` - The same thing as `debugStack` but described in terms of
a native `console.createTask`.
github-actions bot pushed a commit that referenced this pull request Jul 22, 2024
Stacked on #30400 and
#30369

Previously we were using fake evals to recreate a stack for console
replaying and thrown errors. However, for owner stacks we just used the
raw string that came from the server.

This means that the format of the owner stack could include different
formats. Like Spidermonkey format for the client components and V8 for
the server components. This means that this stack can't be parsed
natively by the browser like when printing them as error like in
#30289. Additionally, since
there's no source file registered with that name and no source mapping
url, it can't be source mapped.

Before:

<img width="1329" alt="before-firefox"
src="https://github.com/user-attachments/assets/cbe03f9c-96ac-48fb-b58f-f3a224a774f4">

Instead, we need to create a fake stack like we do for the other things.
That way when it's printed as an Error it gets source mapped. It also
means that the format is consistently in the native format of the
current browser.

After:

<img width="753" alt="after-firefox"
src="https://github.com/user-attachments/assets/b436f1f5-ca37-4203-b29f-df9828c9fad3">

So this is nice because you can just take the result from
`captureOwnerStack()` and append it to an `Error` stack and print it
natively. E.g. this is what React DevTools will do.

If you want to parse and present it yourself though it's a bit awkward
though. The `captureOwnerStack()` API now includes a bunch of
`rsc://React/` URLs. These don't really have any direct connection to
the source map. Only the browser knows this connection from the eval.
You basically have to strip the prefix and then manually pass the
remainder to your own `findSourceMapURL`.

Another awkward part is that since Safari doesn't support eval sourceURL
exposed into `error.stack` - it means that `captureOwnerStack()` get an
empty location for server components since the fake eval doesn't work
there. That's not a big deal since these stacks are already broken even
for client modules for many because the `eval-source-map` strategy in
Webpack doesn't work in Safari for this same reason.

A lot of this refactoring is just clarifying that there's three kind of
ReactComponentInfo fields:

- `stack` - The raw stack as described on the original server.
- `debugStack` - The Error object containing the stack as represented in
the current client as fake evals.
- `debugTask` - The same thing as `debugStack` but described in terms of
a native `console.createTask`.

DiffTrain build for [b15c198](b15c198)
felixshiftellecon added a commit to felixshiftellecon/react that referenced this pull request Jul 24, 2024
Stacked on facebook#30400 and
facebook#30369

Previously we were using fake evals to recreate a stack for console
replaying and thrown errors. However, for owner stacks we just used the
raw string that came from the server.

This means that the format of the owner stack could include different
formats. Like Spidermonkey format for the client components and V8 for
the server components. This means that this stack can't be parsed
natively by the browser like when printing them as error like in
facebook#30289. Additionally, since
there's no source file registered with that name and no source mapping
url, it can't be source mapped.

Before:

<img width="1329" alt="before-firefox"
src="https://github.com/user-attachments/assets/cbe03f9c-96ac-48fb-b58f-f3a224a774f4">

Instead, we need to create a fake stack like we do for the other things.
That way when it's printed as an Error it gets source mapped. It also
means that the format is consistently in the native format of the
current browser.

After:

<img width="753" alt="after-firefox"
src="https://github.com/user-attachments/assets/b436f1f5-ca37-4203-b29f-df9828c9fad3">

So this is nice because you can just take the result from
`captureOwnerStack()` and append it to an `Error` stack and print it
natively. E.g. this is what React DevTools will do.

If you want to parse and present it yourself though it's a bit awkward
though. The `captureOwnerStack()` API now includes a bunch of
`rsc://React/` URLs. These don't really have any direct connection to
the source map. Only the browser knows this connection from the eval.
You basically have to strip the prefix and then manually pass the
remainder to your own `findSourceMapURL`.

Another awkward part is that since Safari doesn't support eval sourceURL
exposed into `error.stack` - it means that `captureOwnerStack()` get an
empty location for server components since the fake eval doesn't work
there. That's not a big deal since these stacks are already broken even
for client modules for many because the `eval-source-map` strategy in
Webpack doesn't work in Safari for this same reason.

A lot of this refactoring is just clarifying that there's three kind of
ReactComponentInfo fields:

- `stack` - The raw stack as described on the original server.
- `debugStack` - The Error object containing the stack as represented in
the current client as fake evals.
- `debugTask` - The same thing as `debugStack` but described in terms of
a native `console.createTask`.
felixshiftellecon added a commit to felixshiftellecon/react that referenced this pull request Jul 24, 2024
This is available outside `enableOwnerStack` but we weren't consistently
using it and so it has different hidden classes.
felixshiftellecon added a commit to felixshiftellecon/react that referenced this pull request Jul 24, 2024
Stacked on facebook#30400 and
facebook#30369

Previously we were using fake evals to recreate a stack for console
replaying and thrown errors. However, for owner stacks we just used the
raw string that came from the server.

This means that the format of the owner stack could include different
formats. Like Spidermonkey format for the client components and V8 for
the server components. This means that this stack can't be parsed
natively by the browser like when printing them as error like in
facebook#30289. Additionally, since
there's no source file registered with that name and no source mapping
url, it can't be source mapped.

Before:

<img width="1329" alt="before-firefox"
src="https://github.com/user-attachments/assets/cbe03f9c-96ac-48fb-b58f-f3a224a774f4">

Instead, we need to create a fake stack like we do for the other things.
That way when it's printed as an Error it gets source mapped. It also
means that the format is consistently in the native format of the
current browser.

After:

<img width="753" alt="after-firefox"
src="https://github.com/user-attachments/assets/b436f1f5-ca37-4203-b29f-df9828c9fad3">

So this is nice because you can just take the result from
`captureOwnerStack()` and append it to an `Error` stack and print it
natively. E.g. this is what React DevTools will do.

If you want to parse and present it yourself though it's a bit awkward
though. The `captureOwnerStack()` API now includes a bunch of
`rsc://React/` URLs. These don't really have any direct connection to
the source map. Only the browser knows this connection from the eval.
You basically have to strip the prefix and then manually pass the
remainder to your own `findSourceMapURL`.

Another awkward part is that since Safari doesn't support eval sourceURL
exposed into `error.stack` - it means that `captureOwnerStack()` get an
empty location for server components since the fake eval doesn't work
there. That's not a big deal since these stacks are already broken even
for client modules for many because the `eval-source-map` strategy in
Webpack doesn't work in Safari for this same reason.

A lot of this refactoring is just clarifying that there's three kind of
ReactComponentInfo fields:

- `stack` - The raw stack as described on the original server.
- `debugStack` - The Error object containing the stack as represented in
the current client as fake evals.
- `debugTask` - The same thing as `debugStack` but described in terms of
a native `console.createTask`.
felixshiftellecon added a commit to felixshiftellecon/react that referenced this pull request Jul 24, 2024
This is available outside `enableOwnerStack` but we weren't consistently
using it and so it has different hidden classes.
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