## Summary
We're experiencing some issues internally where the component stack is
getting into our way of fixing them as it causes the page to become
unresponsive. This adds a flag so that we can disable this feature as a
temporary workaround.
More internal context: https://fburl.com/go9yoklm
## Test Plan
I tried to default this flag to `__VARIANT__` but the variant tests
(`yarn test-www --variant`) started to fail across the board since a lot
of tests depend on the component tree, things like this:
https://user-images.githubusercontent.com/458591/100771192-6a1e1c00-33fe-11eb-9ab0-8ff46ba378a2.png
So, it seems to work :-)
Given that it's unhandy to update the hundreds of tests that are failing
I decided to hard code this to `false` like we already do for some other
options.
* Fixed invalid DevTools work tags
Work tags changed recently (PR #13902) but we didn't bump React versions. This meant that DevTools has valid work tags only for master (and FB www sync) but invalid work tags for the latest open source releases. To fix this, I incremneted React's version in Git (without an actual release) and added a new fork to the work tags detection branch.
This commit also adds tags for the experimental Scope and Fundamental APIs to DevTools so component names will at least display correctly. Technically these new APIs were first introduced to experimental builds ~16.9 but I didn't add a new branch to the work tags fork because I don't they're used commonly. I've just added them to the 17+ branches.
* Removed FundamentalComponent from DevTools tag defs
* Reconcile element types of lazy component yielding the same type
* Add some legacy mode and suspense boundary flushing tests
* Fix infinite loop in legacy mode
In legacy mode we typically commit the suspending fiber and then rerender
the nearest boundary to render the fallback in a separate commit.
We can't do that when the boundary itself suspends because when we try to
do the second pass, it'll suspend again and infinite loop.
Interestingly the legacy semantics are not needed in this case because
they exist to let an existing partial render fully commit its partial state.
In this case there's no partial state, so we can just render the fallback
immediately instead.
* Check fast refresh compatibility first
resolveLazy can suspend and if it does, it can resuspend. Fast refresh
assumes that we don't resuspend. Instead it relies on updating the inner
component later.
* Use timers instead of act to force fallbacks to show
* Rename "name"->"filepath" field on Webpack module references
This field name will get confused with the imported name or the module id.
* Switch back to transformSource instead of getSource
getSource would be more efficient in the cases where we don't need to read
the original file but we'll need to most of the time.
Even then, we can't return a JS file if we're trying to support non-JS
loader because it'll end up being transformed.
Similarly, we'll need to parse the file and we can't parse it before it's
transformed. So we need to chain with other loaders that know how.
* Add acorn dependency
This should be the version used by Webpack since we have a dependency on
Webpack anyway.
* Parse exported names of ESM modules
We need to statically resolve the names that a client component will
export so that we can export a module reference for each of the names.
For export * from, this gets tricky because we need to also load the
source of the next file to parse that. We don't know exactly how the
client is built so we guess it's somewhat default.
* Handle imported names one level deep in CommonJS using a Proxy
We use a proxy to see what property the server access and that will tell
us which property we'll want to import on the client.
* Add export name to module reference and Webpack map
To support named exports each name needs to be encoded as a separate
reference. It's possible with module splitting that different exports end
up in different chunks.
It's also possible that the export is renamed as part of minification.
So the map also includes a map from the original to the bundled name.
* Special case plain CJS requires and conditional imports using __esModule
This models if the server tries to import .default or a plain require.
We should replicate the same thing on the client when we load that
module reference.
* Dedupe acorn-related deps
Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
This convention ensures that you can declare that you intend for a file
to only be used on the server (even if it technically might resolve
on the client).
Previous checks were too naive when it comes to pending lower-pri work or batched updates. This commit adds two new (previously failing) tests and fixes.
This configures the Flight fixture to use the "react-server" environment.
This allows the package.json exports field to specify a different resolution
in this environment.
I use this in the "react" package to resolve to a new bundle that excludes
the Hooks that aren't relevant in this environment like useState and useEffect.
This allows us to error early if these names are imported. If we actually
published ESM, it would we a static error. Now it's a runtime error.
You can test this by importing useState in Container.js which is used
by the client and server.
Until `skipUnmountedBoundaries` lands again, we need some way to detect
when errors are thrown inside a deleted tree. I've added a warning to
`captureCommitPhaseError` that fires when we reach the root of a subtree
without finding either a boundary or a HostRoot.
Even after `skipUnmountedBoundaries` lands, this warning could be a
useful guard against internal bugs, like a bug in the
`skipUnmountedBoundaries` implementation itself.
In the meantime, do not add this warning to the allowlist; this is only
for our internal use. For this reason, I've also only added it to the
new fork, not the old one, to prevent this from accidentally leaking
into the open source build.
Adds a regression test for a bug I found in the effects refactor.
The bug was that reordering a child that contains passive effects would
cause the child to "forget" that it contains passive effects. This is
because when a Placement effect is scheduled by the reconciler, it would
override all of the fiber's flags, including its "static" ones:
```
child.flags = Placement;
```
The problem is that we use a static flag to use a "static" flag to track
that a fiber contains passive effects.
So what happens is that when the tree is deleted, the unmount effect is
never fired.
In the new implementation, the fix is to add the Placement flag without
overriding the rest of the bitmask:
```
child.flags |= Placement;
```
(The old implementation doesn't need to be changed because it does not
use static flags for this purpose.)
* Add Node ESM loader build
This adds a loader build as a first-class export. This will grow in
complexity so it deserves its own module.
* Add Node CommonJS regiter build
This adds a build as a first-class export for legacy CommonJS registration
in Node.js. This will grow in complexity so it deserves its own module.
* Simplify fixture a bit to easier show usage with or without esm
* Bump es version
We leave async function in here which are newer than ES2015.
The last step of the `findDOMNode` algorithm is a search of the
current tree.
When descending into a child node, it mutates `child.return` so that it
points to the current fiber pair, instead of a work-in-progress. This
can cause bugs if `findDOMNode` is called at the wrong time, like in
an interleaved event.
For this reason (among others), you're not suppposed to use
`findDOMNode` in Concurrent Mode. However, we still have some internal
uses that we haven't migrated.
To reduce the potential for bugs, I've removed the `.return` pointer
assignment in favor of recursion.
In the old, effect list implementation, the Deletion flag is is set on
each deleted fiber.
In the new, subtreeTag implementation, the Deletion flag is set on the
parent of each deleted fiber, and the deleted fibers themselves are
pushed to the `deletions` array.
To better distinguish between these two uses, I've added a separate
ChildDeletion flag. That way we can, if desired, maintain both
implementations simultaneously, as we bisect to find the performance
regression that we're currently investigating.
* Fix typo
This typo was fixed in the new fork but not the old.
* Reset new fork to old fork
Something in the new fork is causing a topline metrics regression. We're
not sure what it is, so we're going to split it into steps and bisect.
As a first step, this resets the new fork back to the contents of the
old fork. We will land this to confirm that the fork infra itself is
not causing a regression.
* Fix tests: Add `dfsEffectsRefactor` flag
Some of the tests that gated on the effects refactor used the `new`
flag. In order to bisect, we'll need to decompose the new fork changes
into multiple steps.
So I added a hardcoded test flag called `dfsEffectsRefactor` and set it
to false. Will turn back on when we switch back to traversing the
finished tree using DFS and `subtreeTag`.
Previously this flag was not being reset correctly if a concurrent update followed a nested (sync) update. This PR fixes the behavior and adds a regression test.
When enabled, replaces new fork with old fork.
I've done this several times by manually editing the script file, so
seems useful enough to add an option.
* Pass extra CLI args through to ESLint
These now work:
```
yarn run lint --fix
yarn run linc --fix
```
* Autofix imports when running replace-fork
We have a custom ESLint rule that can autofix cross-fork imports.
Usually, after running the `replace-fork` script, you need to run
`yarn lint --fix` to fix the imports.
This combines the two steps into one.
* Add new effect fields to old fork
So that when comparing relative performance, we don't penalize the new
fork for using more memory.
* Add firstEffect, et al fields to new fork
We need to bisect the changes to the recent commit phase refactor. To
do this, we'll need to add back the effect list temporarily.
This only adds them to the Fiber type so that the memory is the same
as the old fork.
This allows exporting ESM modules for the Webpack plugin. This is necessary
for making a resolver plugin. We could probably make the whole plugin
use ESM instead of CJS ES2015.
My theory for too much inlining contributing to overall stack size is
likely flawed, because Closure reuses variables within a function to
optimize registers.
Even if my theory were correct, the impact would be minimal anyway
because the recursive implementation of the commit phase traversals is
behind a disabled feature flag.
Going to revert this. We can maybe test the impact once we land the
commit phase changes. In the meantime, I'd prefer to eliminate this
delta from the new fork.
This lets the Flight fixture run as "type": "module" or "commonjs".
Experimental loaders can be used similar to require.extensions to do the
transpilation and replacement of .client.js references.
This callback accepts the no parameters (except for the current interactions). Users of this hook can inspect the call stack to access and log the source location of the component.
Bugs caused by inconsistent return pointers are tricky to diagnose
because the source of the error is often in a different part of the
codebase from the actual mistake. For example, you might forget to set a
return pointer during the render phase, which later causes a crash in
the commit phase.
This adds a dev-only invariant to the commit phase to check for
inconsistencies. With this in place, we'll hopefully catch return
pointer errors quickly during local development, when we have the most
context for what might have caused it.
We simulate JSON.stringify in this loop so we should do a has own check.
Otherwise we'll include things like constructor properties.
This will actually make things throw less even when it should.
* Encode Symbols as special rows that can be referenced by models
If a symbol was extracted from Symbol.for(...) then we can reliably
recreate the same symbol on the client.
S123:"react.suspense"
M456:{mySymbol: '$123'}
This doesn't suffer from the XSS problem because you have to write actual
code to create one of these symbols. That problem is only a problem because
values pass through common other usages of JSON which are not secure.
Since React encodes its built-ins as symbols, we can now use them as long
as its props are serializable. Like Suspense.
* Refactor resolution to avoid memo hack
Going through createElement isn't quite equivalent for ref and key in props.
* Reuse symbol ids that have already been written earlier in the stream
* Simplify Relay protocol integration
* Encode Relay rows as tuples instead of objects
This is slightly more compact and more ressembles more closely the encoding
we use for the raw stream protocol.
This ensures that if this server component was the child of a client
component that has an error boundary, it doesn't trigger the error until
this gets rendered so it happens as deep as possible.
* Expand fixture
Use .server convention. /server/index.js should really change too so it can be compiled but for now we treat it as bootstrapping code outside the compiled code.
Move App.server. It's part of the application code rather than the infra.
Add hybrid component used in both server/client and an extra component shared by multiple entry points.
* Use require.extensions to replace .client imports
The simplest server doesn't need AOT compilation. Instead we can just
configure require.extensions. This is probably not the best idea to use
in prod but is enough to show the set up.
* Do not fix return pointers during commit phase
In the commit phase, we should be able to assume that the `return`
pointers in the just-completed tree are consistent. The render phase
should be responsible for ensuring these are always correct.
I've removed the `return` pointer assignments from the render phase
traversal logic. This isn't all of them, only the ones added recently
during the effects refactor. The other ones have been around longer so
I'll leave those for a later clean up.
This breaks a few SuspenseList tests; I'll fix in the next commit.
* Set return pointer when reusing current tree
We always set the return pointer on freshly cloned, work-in-progress
fibers. However, we were neglecting to set them on trees that are reused
from current.
I fixed this in the same path of the complete phase where we reset the
fiber flags.
This is a code smell because it assumes the commit phase is never
concurrent with the render phase. Our eventual goal is to make fibers a
lock free data structure.
Will address further during refactor to alternate model.
Background:
State updates that are scheduled in a layout effect (useLayoutEffect or componentDidMount / componentDidUpdate) get processed synchronously by React before it yields to the browser to paint. This is done so that components can adjust their layout (e.g. position and size a tooltip) without any visible shifting being seen by users. This type of update is often called a "nested update" or a "cascading update".
Because they delay paint, nested updates are considered expensive and should be avoided when possible. For example, effects that do not impact layout (e.g. adding event handlers, logging impressions) can be safely deferred to the passive effect phase by using useEffect instead.
This PR updates the Profiler API to explicitly flag nested updates so they can be monitored for and avoided when possible.
Implementation:
I considered a few approaches for this.
Add a new callback (e.g. onNestedUpdateScheduled) to the Profiler that gets called when a nested updates gets scheduled.
Add an additional boolean parameter to the end of existing callbacks (e.g. wasNestedUpdate).
Update the phase param to add an additional variant: "mount", "update", or "nested-update" (new).
I think the third option makes for the best API so that's what I've implemented in this PR.
Because the Profiler API is stable, this change will need to remain behind a feature flag until v18. I've turned the feature flag on for Facebook builds though after confirming that Web Speed does not currently make use of the phase parameter.
Quirks:
One quirk about the implementation I've chosen is that errors thrown during the layout phase are also reported as nested updates. I believe this is appropriate since these errors get processed synchronously and block paint. Errors thrown during render or from within passive effects are not affected by this change.
This reverts commits bcca5a6ca7 and ffb749c95e, although neither revert cleanly since methods have been moved between the work-loop and commit-work files. This commit is a mostly manual effort of undoing the changes.
Philipp Spiess
Dan Abramov
Sebastian Markbåge
Brian Vaughn
Andrew Clark
Paul Doyle
Dan Abramov
Billy Janitsch
Alphabet Codes
Ricky
Sean Keenan
inokawa
Jack Works
