[Fiber] Warn for Conditional Use of use() Based on Cache - #37104
Conversation
|
Comparing: 0f42eac...5c35f9f Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
| }); | ||
|
|
||
| // @gate __DEV__ && enableConditionalUseWarning | ||
| it('warns if use(promise) is called conditionally based on a cache', async () => { |
There was a problem hiding this comment.
Nit: could also use a test asserting that warning does not fire on correct usage, though thats likely covered by other use tests. And maybe rerender this at the end to make sure warning doesn't fire again.
There was a problem hiding this comment.
could also use a test asserting that warning does not fire on correct usage
That would be complicated, because our gating mechanism for tests asserts that any test run that doesn't pass the check should fail. In other words, the base scenario should fail, but this won't be true for the behavior that will be tested.
There was a problem hiding this comment.
Actually, I could just remove these gating conditions
| 'did not call use() when it finished. This indicates an incorrect use of use(). ' + | ||
| 'A common mistake is to call use() only when something is not cached.\n\n' + | ||
| ' if (cache.value !== undefined) use(cache.promise) else return cache.value\n\n' + | ||
| 'The correct way is to always call use() with a Promise and resolve it with the value.\n\n' + |
There was a problem hiding this comment.
We should follow up with documenting the fulfilled optimization somewhere
There was a problem hiding this comment.
The docs team added in a note about this optimization with the following pitfall.
7de52db to
b95fba5
Compare
b95fba5 to
7c5c268
Compare
7c5c268 to
fa0f78d
Compare
|
@eps1lon Had a merge conflict, need a stamp again :( |
fa0f78d to
9a87d38
Compare
| lastSuspendedStack = new Error( | ||
| 'This library called use() to suspend in a previous render but ' + | ||
| 'did not call use() when it finished. This indicates an incorrect use of use(). ' + | ||
| 'Learn more: https://react.dev/warnings/conditional-use-of-use.', | ||
| ); |
There was a problem hiding this comment.
Amended warning message, added a link to docs. See reactjs/react.dev#8567.
9a87d38 to
5c35f9f
Compare
6 participants
This is a cherry-pick of #34030, with a feature flag gating and a test coverage.
The flag is disabled by default and dynamic for FB builds to understand first how noisy this warning can be.
See #34030 for more context on the change.