Passive flags are a new concept that is tricky to get right. We've
already found two bugs related to PassiveStatic. Let's remove this
optimization for now, and add it back once the main part of the effects
refactor lands.
As per Seb's comment in #20465, we need to do the same thing in React Native as we do in Relay.
When `parseModel` suspends because of missing dependencies, it will exit and retry to parse later. However, in the relay implementation, the model is an object that we modify in place when we parse it, so when we we retry, part of the model might be parsed already into React elements, which will error because the parsing code expect a Flight model. This diff clones instead of mutating the original model, which fixes this error.
When `parseModel` suspends because of missing dependencies, it will exit and retry to parse later. However, in the relay implementation, the model is an object that we modify in place when we parse it, so when we we retry, part of the model might be parsed already into React elements, which will error because the parsing code expect a Flight model. This diff clones instead of mutating the original model, which fixes this error.
@sebmarkbage reminded me that the complete phase of SuspenseList
will sometimes enter the begin phase of the children without calling
`createWorkInProgress` again, instead calling `resetWorkInProgress`.
This was raised in the context of considering whether #20398 might
have accidentally caused a SuspenseList bug. (I did look at this code
at the time, but considering how complicated SuspenseList is it's not
hard to imagine that I made a mistake.)
Anyway, I think that PR is fine; however, reviewing it again did lead me
to find a different bug. This new bug is actually a variant of the bug
fixed by #20398.
`resetWorkInProgress` clears a fiber's static flags. That's wrong, since
static flags -- like PassiveStatic -- are meant to last the lifetime of
the component.
In more practical terms, what happens is that if a direct child of
SuspenseList contains a `useEffect`, then SuspenseList will cause the
child to "forget" that it contains passive effects. When the child
is deleted, its unmount effects are not fired :O
This is the second of this type of bug I've found, which indicates to me
that it's too easy to accidentally clear static flags.
Maybe we should only update the `flags` field using helper functions,
like we do with `lanes`.
Or perhaps we add an internal warning somewhere that detects when a
fiber has different static flags than its alternate.
We replay errors so you can break on paused exceptions. This is done in
the second pass so that the first pass can ignore suspense.
Originally this threw the original error. For suppression purposes
we copied the flag onto the original error.
f1dc626b29/packages/react-reconciler/src/ReactFiberScheduler.old.js (L367-L369)
During this refactor it changed to just throw the retried error:
https://github.com/facebook/react/pull/15151
We're not sure exactly why but it was likely just an optimization or
clean up.
So we can go back to throwing the original error. That helps in the case
where a memoized function is naively not rethrowing each time such as
in Node's module system.
Unfortunately this doesn't fix the problem fully.
Because invokeGuardedCallback captures the error and logs it to the browser.
So you still end up seeing the wrong message in the logs.
This just fixes so that the error boundary sees the first one.
* Don't allocate the inner cache unnecessarily
We only need it when we're asking for text. I anticipate I'll want to avoid allocating it in other methods too when it's not strictly necessary.
* Add fs.access
* Add fs.lstat
* Add fs.stat
* Add fs.readdir
* Add fs.readlink
* Add fs.realpath
* Rename functions to disambiguate two caches
We originally added a new DEV behavior of double-invoking effects during mount to our new reconciler fork in PRs #19523 and #19935 and later refined it to only affect modern roots in PR #20028. This PR adds that behavior to the old reconciler fork with a small twist– the behavior applies to StrictMode subtrees, regardless of the root type.
This commit also adds a few additional tests that weren't in the original commits.
* [Flight] Add rudimentary FS binding
* Throw for unsupported
* Don't mess with hidden class
* Use absolute path as the key
* Warn on relative and non-normalized paths
This was added in a later step of the refactor but since `deletions`
array already landed, clearing it should, too.
I think it's unlikely that this causes GC problems but worth
adding anyway.
* Move files
* Update paths
* Rename import variables
* Rename /server to /writer
This is mainly because "React Server Server" is weird so we need another
dimension.
* Use "react-server" convention to enforce that writer is only loaded in a server
* Bump all versions
* Switch to CJS mode
* Revert "Switch to CJS mode"
This reverts commit b3c4fd92dcef6ecb4116fc66f674ae88aad3c582.
* Fix ES mode
* Add nodemon to restart the server on edits
* Ignore /server/ from compilation
Adds back the `deletions` array and uses it in the commit phase.
We use a trick where the first time we hit a deletion effect, we commit
all the deletion effects that belong to that parent. This is an
incremental step away from using the effect list and toward a DFS +
subtreeFlags traversal.
This will help determine whether the regression is caused by, say,
pushing the same fiber into the deletions array multiple times.
When scheduling a Placement effect, we should add the Placement bit
without resetting the others.
In the old fork, there are no flags to reset, anyway, since by the
time we reach the child reconciler, the flags will have already been
reset.
However, in the effects refactor, "static" flags are not reset, so this
can actually manifest as a bug. See #20285 for a regression test.
* Basic scan of the file system to find Client modules
This does a rudimentary merge of the plugins. It still uses the global
scan and writes to file system.
Now the plugin accepts a search path or a list of referenced client files.
In prod, the best practice is to provide a list of files that are actually
referenced rather than including everything possibly reachable. Probably
in dev too since it's faster.
This is using the same convention as the upstream ContextModule - which
powers the require.context helpers.
* Add neo-async to dependencies
* Don't use async/await
Babel transpilation fails for some reason in prod.
* Set up production runner command
Uses python because meh. Just to show it's static.
* Use build folder in prod
This wasn't forked previously because Lane and associated types are
opaque, and they leak into non-reconciler packages. So forking the type
would also require forking all those other packages.
But I really want to use the reconciler fork infra for lanes changes.
So I made them no longer opaque.
Another possible solution would be to add separate `new` and `old`
fields to the Fiber type, like I did when migrating from expiration
times. But that seems so excessive. This seems fine.
But we should still treat them like they're opaque and only do lanes
manipulation in the ReactFiberLane module. At least until the model
stabilizes more. We'll just need to enforce this with discipline
instead of with the type system.
