Export captureOwnerStacks() only in DEV "react" builds

[sebmarkbage]

Only with the enableOwnerStacks flag (which is not on in www).

This is a new DEV-only API to be able to implement what we do for console.error in user space.

This API does not actually include the current stack that you'd get from new Error().stack. That you'd have to add yourself.

This adds the ability to have conditional development exports because we plan on eventually having separate ESM builds that use the "development" or "production" export conditions.

NOTE: This removes the export of act from react in prod (as opposed to a function that throws) - inline with what we do with other conditional exports.

[vercel]

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	Jun 18, 2024 2:11pm

[react-sizebot]

Comparing: f3e09d6...98dc7fb

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.66 kB	6.66 kB	=	1.82 kB	1.82 kB
oss-stable/react-dom/cjs/react-dom-client.production.js	=	497.80 kB	497.80 kB	=	89.24 kB	89.24 kB
oss-experimental/react-dom/cjs/react-dom.production.js	=	6.67 kB	6.67 kB	+0.05%	1.83 kB	1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js	=	502.62 kB	502.62 kB	=	89.94 kB	89.94 kB
facebook-www/ReactDOM-prod.classic.js	=	597.04 kB	597.04 kB	=	105.31 kB	105.31 kB
facebook-www/ReactDOM-prod.modern.js	=	571.38 kB	571.38 kB	=	101.25 kB	101.25 kB
test_utils/ReactAllWarnings.js	Deleted	63.50 kB	0.00 kB	Deleted	15.90 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-experimental/react/cjs/react.react-server.development.js	+0.51%	37.15 kB	37.34 kB	+0.40%	8.76 kB	8.80 kB
oss-experimental/react/cjs/react.development.js	+0.41%	46.60 kB	46.79 kB	+0.27%	10.66 kB	10.69 kB
oss-experimental/react/cjs/react.production.js	+0.34%	17.91 kB	17.97 kB	+0.34%	4.65 kB	4.67 kB
oss-experimental/react/cjs/react.react-server.production.js	+0.32%	18.61 kB	18.67 kB	+0.34%	4.95 kB	4.97 kB
test_utils/ReactAllWarnings.js	Deleted	63.50 kB	0.00 kB	Deleted	15.90 kB	0.00 kB

Generated by 🚫 dangerJS against 98dc7fb

[phryneas]

So, this could be used in a test environment (if using a node build of React) by testing utilities to get the name of the current component?

This would help us to not access React internals in some of our tests :)
(for comparison: https://github.com/apollographql/apollo-client/blob/a739dfd2847d002405bb6c8b637ead9e54d5d012/src/testing/internal/profile/profile.tsx#L449C1-L469C2 )

[sebmarkbage]

@phryneas Well, yes and no. One of the concerns about this API surface that I'm concerned about is that people end up building "enzyme"-style testing that assert on the implementation details of the above tree.

Notably this doesn't actually give you the name of the current component because the nearest is the callsite that created this component. You'd have to get new Error().stack to get the current one.

The problem with that is not just that it uses internals but if you assert on the implementation details of another component that can change its internals an split into multiple components or merge. Which might even include React internals themselves. We're actually planning on changing forwardRef, memo etc. to be split into user component and so the owner stack involving those would change.

The test selectors API we've designed is more designed in such a way that you can only assert on implementation details that are public anyway.

Where as this can inspect things that are much less stable.

So depending on how you use it it might be a bad idea anyway. If you just use it to print better error messages when something fails for other reasons though, yea that's a good use case for it.

[phryneas]

@phryneas Well, yes and no. One of the concerns about this API surface that I'm concerned about is that people end up building "enzyme"-style testing that assert on the implementation details of the above tree.

Guilty 😅
Well, honestly, not really. I don't want to derail the PR, but I'd also be very happy about feedback here:
We found that for testing a library, the "eventual consistency" style of RTL-testing doesn't really work.
While the test is waiting for a consistent result, we could inadvertedly rerender 20 times with inconsistent return values in-between, and our tests wouldn't catch it.
While that is usually fine in UIs, having that happen in a library... well, every single accidental rerender we perform trickles down to thousands of applications, and an inconsistent render would cause tons of userland bugs.
So, as a result, most of our newer tests use the <Profiler> component and every time a React render happened, we take a snapshot - depending of the test, we sometimes snapshot a DOM, sometimes we snapshot hook return values, sometimes other things.
Especially in our Suspense hooks tests, it has been proven invaluable that we also track which components actively participated in a specific render. Tracking those is a bit fiddly at the moment, but we make do. (Keep in mind, we're testing a hook library, so every test defines a bunch of components and then we test how they interact - we can just add something like useTrackRender() to each component.)
Then, our tests consume all these snapshots in an async-iterator-like pattern, so we can make assumptions on every single render.

Examples on how those tests look:

• https://github.com/apollographql/apollo-client/blob/25ff067b36bd4ec3833a2d71f74c8aed75e2944f/src/react/hooks/__tests__/useQuery.test.tsx#L5187-L5216
• https://github.com/apollographql/apollo-client/blob/6b39e3f19ba0944b38c8b2822945dc7ca847731c/src/react/hooks/__tests__/useSuspenseQuery.test.tsx#L361-L415
• https://github.com/apollographql/apollo-client/blob/6b39e3f19ba0944b38c8b2822945dc7ca847731c/src/react/hooks/__tests__/useLoadableQuery.test.tsx#L307-L356

From your feedback up there, this seems to be the wrong API for us to use, so I'll stay away from it.

But I have to wonder: As you seem to be adding a few interesting APIs, could this be something we could chat about a bit?
I've looked into recording profiler traces like the devtools do, but that essentially requires spinning up a lot of interprocess communication stuff that is necessary for the devtools, but would not be necessary for a "library testing library".
Maybe you could expose an API for something like this, too?

I've seen a few calls on Twitter recently that React should be more performance testing with the argument that the "suspense sibling change" impact could have been caught sooner that way.
I guess something like that would probably be hard to set up for you, but if we had good tools to closely monitor library behaviour in our tests, it would make it more easy for libraries to notice these changes in canary builds and report back to you.

[sebmarkbage]

I've seen a few calls on Twitter recently that React should be more performance testing with the argument that the "suspense sibling change" impact could have been caught sooner that way.

IMO some of the communication undersold how much we knew about this already and did the call anyway. It's tradeoffs and time constraints. Regardless testing React itself against other libraries is something we can do easily anyway and we get coverage from real products. It's also overvaluing status quo (which is my problem with the whole thing). If you can never regress you must almost by definition end up in a local maxima which is where React is heading.

It doesn't necessarily affect testing of most user code. Like React need to be able to change some implementation details like this without breaking all user tests. So there's a risk of an over reliance on implementation details. Like imagine if every manual useEffect+fetch encoded these semantics then upgrading would be a blocker just due to the tests.

For the library use case like yours it's a little different. You're more concerned about details and if React makes a breaking change then you'd actually be motivated to fix the tests and workaround a change. But then also relying on internals is maybe not the end of the world because breaking your tests isn't going to break your downstream users. But feel free to open a different issue about that.