* Add fixture for comparing baseline render perf for renderToString and renderToPipeableStream
Modified from ssr2 and https://github.com/SuperOleg39/react-ssr-perf-test
* Implement buffering in pipeable streams
The previous implementation of pipeable streaming (Node) suffered some performance issues brought about by the high chunk counts and innefficiencies with how node streams handle this situation. In particular the use of cork/uncork was meant to alleviate this but these methods do not do anything unless the receiving Writable Stream implements _writev which many won't.
This change adopts the view based buffering techniques previously implemented for the Browser execution context. The main difference is the use of backpressure provided by the writable stream which is not implementable in the other context. Another change to note is the use of standards constructs like TextEncoder and TypedArrays.
* Implement encodeInto during flushCompletedQueues
encodeInto allows us to write directly to the view buffer that will end up getting streamed instead of encoding into an intermediate buffer and then copying that data.
* Implement addEventListener and removeEventListener on Fabric HostComponent
* add files
* re-add CustomEvent
* fix flow
* Need to get CustomEvent from an import since it won't exist on the global scope by default
* yarn prettier-all
* use a mangled name consistently to refer to imperatively registered event handlers
* yarn prettier-all
* fuzzy null check
* fix capture phase event listener logic
* early exit from getEventListeners more often
* make some optimizations to getEventListeners and the bridge plugin
* fix accumulateInto logic
* fix accumulateInto
* Simplifying getListeners at the expense of perf for the non-hot path
* feedback
* fix impl of getListeners to correctly remove function
* pass all args in to event listeners
* RawEventEmitter: new event perf profiling mechanism outside of Pressability to capture all touch events, and other event types
* sync
* concise notation
* Move event telemetry event emitter call from Plugin to ReactFabricEventEmitter, to reduce reliance on the plugin system and move the emit call further into the core
* Backout changes to ReactNativeEventPluginOrder
* Properly flow typing event emitter, and emit event to two channels: named and catchall
* fix typing for event name string
* fix typing for event name string
* fix flow
* Add more comments about how the event telemetry system works
* Add more comments about how the event telemetry system works
* rename to RawEventTelemetryEventEmitterOffByDefault
* yarn prettier-all
* rename event
* comments
* improve flow types
* renamed file
* [RFC] Add onHydrationError option to hydrateRoot
This is not the final API but I'm pushing it for discussion purposes.
When an error is thrown during hydration, we fallback to client
rendering, without triggering an error boundary. This is good because,
in many cases, the UI will recover and the user won't even notice that
something has gone wrong behind the scenes.
However, we shouldn't recover from these errors silently, because the
underlying cause might be pretty serious. Server-client mismatches are
not supposed to happen, even if UI doesn't break from the users
perspective. Ignoring them could lead to worse problems later. De-opting
from server to client rendering could also be a significant performance
regression, depending on the scope of the UI it affects.
So we need a way to log when hydration errors occur.
This adds a new option for `hydrateRoot` called `onHydrationError`. It's
symmetrical to the server renderer's `onError` option, and serves the
same purpose.
When no option is provided, the default behavior is to schedule a
browser task and rethrow the error. This will trigger the normal browser
behavior for errors, including dispatching an error event. If the app
already has error monitoring, this likely will just work as expected
without additional configuration.
However, we can also expose additional metadata about these errors, like
which Suspense boundaries were affected by the de-opt to client
rendering. (I have not exposed any metadata in this commit; API needs
more design work.)
There are other situations besides hydration where we recover from an
error without surfacing it to the user, or notifying an error boundary.
For example, if an error occurs during a concurrent render, it could be
due to a data race, so we try again synchronously in case that fixes it.
We should probably expose a way to log these types of errors, too. (Also
not implemented in this commit.)
* Log all recoverable errors
This expands the scope of onHydrationError to include all errors that
are not surfaced to the UI (an error boundary). In addition to errors
that occur during hydration, this also includes errors that recoverable
by de-opting to synchronous rendering. Typically (or really, by
definition) these errors are the result of a concurrent data race;
blocking the main thread fixes them by prevents subsequent races.
The logic for de-opting to synchronous rendering already existed. The
only thing that has changed is that we now log the errors instead of
silently proceeding.
The logging API has been renamed from onHydrationError
to onRecoverableError.
* Don't log recoverable errors until commit phase
If the render is interrupted and restarts, we don't want to log the
errors multiple times.
This change only affects errors that are recovered by de-opting to
synchronous rendering; we'll have to do something else for errors
during hydration, since they use a different recovery path.
* Only log hydration error if client render succeeds
Similar to previous step.
When an error occurs during hydration, we only want to log it if falling
back to client rendering _succeeds_. If client rendering fails,
the error will get reported to the nearest error boundary, so there's
no need for a duplicate log.
To implement this, I added a list of errors to the hydration context.
If the Suspense boundary successfully completes, they are added to
the main recoverable errors queue (the one I added in the
previous step.)
* Log error with queueMicrotask instead of Scheduler
If onRecoverableError is not provided, we default to rethrowing the
error in a separate task. Originally, I scheduled the task with
idle priority, but @sebmarkbage made the good point that if there are
multiple errors logs, we want to preserve the original order. So I've
switched it to a microtask. The priority can be lowered in userspace
by scheduling an additional task inside onRecoverableError.
* Only use host config method for default behavior
Redefines the contract of the host config's logRecoverableError method
to be a default implementation for onRecoverableError if a user-provided
one is not provided when the root is created.
* Log with reportError instead of rethrowing
In modern browsers, reportError will dispatch an error event, emulating
an uncaught JavaScript error. We can do this instead of rethrowing
recoverable errors in a microtask, which is nice because it avoids any
subtle ordering issues.
In older browsers and test environments, we'll fall back
to console.error.
* Naming nits
queueRecoverableHydrationErrors -> upgradeHydrationErrorsToRecoverable
* Revise ESLint rules for string coercion
Currently, react uses `'' + value` to coerce mixed values to strings.
This code will throw for Temporal objects or symbols.
To make string-coercion safer and to improve user-facing error messages,
This commit adds a new ESLint rule called `safe-string-coercion`.
This rule has two modes: a production mode and a non-production mode.
* If the `isProductionUserAppCode` option is true, then `'' + value`
coercions are allowed (because they're faster, although they may
throw) and `String(value)` coercions are disallowed. Exception:
when building error messages or running DEV-only code in prod
files, `String()` should be used because it won't throw.
* If the `isProductionUserAppCode` option is false, then `'' + value`
coercions are disallowed (because they may throw, and in non-prod
code it's not worth the risk) and `String(value)` are allowed.
Production mode is used for all files which will be bundled with
developers' userland apps. Non-prod mode is used for all other React
code: tests, DEV blocks, devtools extension, etc.
In production mode, in addiiton to flagging `String(value)` calls,
the rule will also flag `'' + value` or `value + ''` coercions that may
throw. The rule is smart enough to silence itself in the following
"will never throw" cases:
* When the coercion is wrapped in a `typeof` test that restricts to safe
(non-symbol, non-object) types. Example:
if (typeof value === 'string' || typeof value === 'number') {
thisWontReport('' + value);
}
* When what's being coerced is a unary function result, because unary
functions never return an object or a symbol.
* When the coerced value is a commonly-used numeric identifier:
`i`, `idx`, or `lineNumber`.
* When the statement immeidately before the coercion is a DEV-only
call to a function from shared/CheckStringCoercion.js. This call is a
no-op in production, but in DEV it will show a console error
explaining the problem, then will throw right after a long explanatory
code comment so that debugger users will have an idea what's going on.
The check function call must be in the following format:
if (__DEV__) {
checkXxxxxStringCoercion(value);
};
Manually disabling the rule is usually not necessary because almost all
prod use of the `'' + value` pattern falls into one of the categories
above. But in the rare cases where the rule isn't smart enough to detect
safe usage (e.g. when a coercion is inside a nested ternary operator),
manually disabling the rule will be needed.
The rule should also be manually disabled in prod error handling code
where `String(value)` should be used for coercions, because it'd be
bad to throw while building an error message or stack trace!
The prod and non-prod modes have differentiated error messages to
explain how to do a proper coercion in that mode.
If a production check call is needed but is missing or incorrect
(e.g. not in a DEV block or not immediately before the coercion), then
a context-sensitive error message will be reported so that developers
can figure out what's wrong and how to fix the problem.
Because string coercions are now handled by the `safe-string-coercion`
rule, the `no-primitive-constructor` rule no longer flags `String()`
usage. It still flags `new String(value)` because that usage is almost
always a bug.
* Add DEV-only string coercion check functions
This commit adds DEV-only functions to check whether coercing
values to strings using the `'' + value` pattern will throw. If it will
throw, these functions will:
1. Display a console error with a friendly error message describing
the problem and the developer can fix it.
2. Perform the coercion, which will throw. Right before the line where
the throwing happens, there's a long code comment that will help
debugger users (or others looking at the exception call stack) figure
out what happened and how to fix the problem.
One of these check functions should be called before all string coercion
of user-provided values, except when the the coercion is guaranteed not
to throw, e.g.
* if inside a typeof check like `if (typeof value === 'string')`
* if coercing the result of a unary function like `+value` or `value++`
* if coercing a variable named in a whitelist of numeric identifiers:
`i`, `idx`, or `lineNumber`.
The new `safe-string-coercion` internal ESLint rule enforces that
these check functions are called when they are required.
Only use these check functions in production code that will be bundled
with user apps. For non-prod code (and for production error-handling
code), use `String(value)` instead which may be a little slower but will
never throw.
* Add failing tests for string coercion
Added failing tests to verify:
* That input, select, and textarea elements with value and defaultValue
set to Temporal-like objects which will throw when coerced to string
using the `'' + value` pattern.
* That text elements will throw for Temporal-like objects
* That dangerouslySetInnerHTML will *not* throw for Temporal-like
objects because this value is not cast to a string before passing to
the DOM.
* That keys that are Temporal-like objects will throw
All tests above validate the friendly error messages thrown.
* Use `String(value)` for coercion in non-prod files
This commit switches non-production code from `'' + value` (which
throws for Temporal objects and symbols) to instead use `String(value)`
which won't throw for these or other future plus-phobic types.
"Non-produciton code" includes anything not bundled into user apps:
* Tests and test utilities. Note that I didn't change legacy React
test fixtures because I assumed it was good for those files to
act just like old React, including coercion behavior.
* Build scripts
* Dev tools package - In addition to switching to `String`, I also
removed special-case code for coercing symbols which is now
unnecessary.
* Add DEV-only string coercion checks to prod files
This commit adds DEV-only function calls to to check if string coercion
using `'' + value` will throw, which it will if the value is a Temporal
object or a symbol because those types can't be added with `+`.
If it will throw, then in DEV these checks will show a console error
to help the user undertsand what went wrong and how to fix the
problem. After emitting the console error, the check functions will
retry the coercion which will throw with a call stack that's easy (or
at least easier!) to troubleshoot because the exception happens right
after a long comment explaining the issue. So whether the user is in
a debugger, looking at the browser console, or viewing the in-browser
DEV call stack, it should be easy to understand and fix the problem.
In most cases, the safe-string-coercion ESLint rule is smart enough to
detect when a coercion is safe. But in rare cases (e.g. when a coercion
is inside a ternary) this rule will have to be manually disabled.
This commit also switches error-handling code to use `String(value)`
for coercion, because it's bad to crash when you're trying to build
an error message or a call stack! Because `String()` is usually
disallowed by the `safe-string-coercion` ESLint rule in production
code, the rule must be disabled when `String()` is used.
This removes all the remaining references to the `build2` directory
except for the CI job that stores the artifacts. We'll keep the
`build2` artifact until downstream scripts are migrated to `build`.
React currently suppress console logs in StrictMode during double rendering. However, this causes a lot of confusion. This PR moves the console suppression logic from React into React Devtools. Now by default, we no longer suppress console logs. Instead, we gray out the logs in console during double render. We also add a setting in React Devtools to allow developers to hide console logs during double render if they choose.
## Summary
Adds support for statically extracting names for hook calls from source code, and extending source maps with that information so that DevTools does not have to directly parse source code at runtime, which will speed up the Named Hooks feature and allow it to be enabled by default.
Specifically, this PR includes the following parts:
- [x] Adding logic to statically extract relevant hook names from the parsed source code (i.e. the babel ast). Note that this logic differs slightly from the existing logic in that the existing logic also uses runtime information from DevTools (such as whether given hooks are a custom hook) to extract names for hooks, whereas this code is meant to run entirely at build time, so it does not rely on that information.
- [x] Generating an encoded "hook map", which encodes the information about a hooks *original* source location, and it's corresponding name. This "hook map" will be used to generate extended source maps, included tentatively under an extra `x_react_hook_map` field. The map itself is formatted and encoded in a very similar way as how the `names` and `mappings` fields of a standard source map are encoded ( = Base64 VLQ delta coding representing offsets into a string array), and how the "function map" in Metro is encoded, as suggested in #21782. Note that this initial version uses a very basic format, and we are not implementing our own custom encoding, but reusing the `encode` function from `sourcemap-codec`.
- [x] Updating the logic in `parseHookNames` to check if the source maps have been extended with the hook map information, and if so use that information to extract the hook names without loading the original source code. In this PR we are manually generating extended source maps in our tests in order to test that this functionality works as expected, even though we are not actually generating the extended source maps in production.
The second stage of this work, which will likely need to occur outside this repo, is to update bundlers such as Metro to use these new primitives to actually generate source maps that DevTools can use.
### Follow-ups
- Enable named hooks by default when extended source maps are present
- Support looking up hook names when column numbers are not present in source map.
- Measure performance improvement of using extended source maps (manual testing suggests ~4 to 5x faster)
- Update relevant bundlers to generate extended source maps.
## Test Plan
- yarn flow
- Tests still pass
- yarn test
- yarn test-build-devtools
- Named hooks still work on manual test of browser extension on a few different apps (code sandbox, create-react-app, facebook).
- For new functionality:
- New tests for statically extracting hook names.
- New tests for using extended source maps to look up hook names at runtime.
* Re-add old Fabric Offscreen impl behind flag
There's a chance that #21960 will affect layout in a way that we don't
expect, so I'm adding back the old implementation so we can toggle the
feature with a flag.
The flag should read from the ReactNativeFeatureFlags shim so that we
can change it at runtime. I'll do that separately.
* Import dynamic RN flags from external module
Internal feature flags that we wish to control with a GK can now be
imported from an external module, which I've called
"ReactNativeInternalFeatureFlags".
We'll need to add this module to the downstream repo.
We can't yet use this in our tests, because we don't have a test
configuration that runs against the React Native feature flags fork. We
should set up that up the same way we did for www.
* Call into Fabric to get current event priority
Fix flow errors
* Prettier
* Better handle null and undefined cases
* Remove optional chaining and use ?? operator
* prettier-all
* Use conditional ternary operator
* prettier
With this change, if a node is a Fabric node, we route the setJSResponder call to FabricUIManager. Native counterpart is already landed. Tested internally as D26241364.
* RN: Implement `sendAccessibilityEvent` on HostComponent
Implement `sendAccessibilityEvent` on HostComponent for Fabric and non-Fabric RN.
Currently the Fabric version is a noop and non-Fabric uses
AccessibilityInfo directly. The Fabric version will be updated once
native Fabric Android/iOS support this method in the native UIManager.
* Move methods out of HostComponent
* Properly type dispatchCommand and sendAccessibilityEvent handle arg
* Implement Fabric side of sendAccessibilityEvent
* Add tests: 1. Fabric->Fabric, 2. Paper->Fabric, 3. Fabric->Paper, 4. Paper->Paper
* Fix typo: ReactFaricEventTouch -> ReactFabricEventTouch
* fix flow types
* prettier
* 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
* [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
* 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>
* 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 adds a new dimension similar to dom-relay. It's different from
"native" which would be Flight for RN without Relay.
This has some copy-pasta that's the same between the two Relay builds but
the key difference will be Metro and we're not quite sure what other
differences there will be yet.
* Refactor Flight to require a module reference to be brand checked
This exposes a host environment (bundler) specific hook to check if an
object is a module reference. This will be used so that they can be passed
directly into Flight without needing additional wrapper objects.
* Emit module references as a special type of value
We already have JSON and errors as special types of "rows". This encodes
module references as a special type of row value. This was always the
intention because it allows those values to be emitted first in the stream
so that as a large models stream down, we can start preloading as early
as possible.
We preload the module when they resolve but we lazily require them as they
are referenced.
* Emit module references where ever they occur
This emits module references where ever they occur. In blocks or even
directly in elements.
* Don't special case the root row
I originally did this so that a simple stream is also just plain JSON.
However, since we might want to emit things like modules before the root
module in the stream, this gets unnecessarily complicated. We could add
this back as a special case if it's the first byte written but meh.
* Update the protocol
* Add test for using a module reference as a client component
* Relax element type check
Since Flight now accepts a module reference as returned by any bundler
system, depending on the renderer running. We need to drastically relax
the check to include all of them. We can add more as we discover them.
* Move flow annotation
Seems like our compiler is not happy with stripping this.
* Some bookkeeping bug
* Can't use the private field to check
We typecheck the reconciler against each one of our host configs.
`yarn flow dom` checks it against the DOM renderer, `yarn flow native`
checks it against the native renderer, and so on.
To do this, we generate separate flowconfig files.
Currently, there is no root-level host config, so running Flow
directly via `flow` CLI doesn't work. You have to use the `yarn flow`
command and pick a specific renderer.
A drawback of this design, though, is that our Flow setup doesn't work
with other tooling. Namely, editor integrations.
I think the intent of this was maybe so you don't run Flow against a
renderer than you intended, see it pass, and wrongly think you fixed
all the errors. However, since they all run in CI, I don't think this
is a big deal. In practice, I nearly always run Flow against the same
renderer (DOM), and I'm guessing that's the most common workflow for
others, too.
So what I've done in this commit is modify the `yarn flow` command to
copy the generated `.flowconfig` file into the root directory. The
editor integration will pick this up and show Flow information for
whatever was the last renderer you checked.
Everything else about the setup is the same, and all the renderers will
continue to be checked by CI.
Some of our internal reconciler types have leaked into other packages.
Usually, these types are treated as opaque; we don't read and write
to its fields. This is good.
However, the type is often passed back to a reconciler method. For
example, React DOM creates a FiberRoot with `createContainer`, then
passes that root to `updateContainer`. It doesn't do anything with the
root except pass it through, but because `updateContainer` expects a
full FiberRoot, React DOM is still coupled to all its fields.
I don't know if there's an idiomatic way to handle this in Flow. Opaque
types are simlar, but those only work within a single file. AFAIK,
there's no way to use a package as the boundary for opaqueness.
The immediate problem this presents is that the reconciler refactor will
involve changes to our internal data structures. I don't want to have to
fork every single package that happens to pass through a Fiber or
FiberRoot, or access any one of its fields. So my current plan is to
share the same Flow type across both forks. The shared type will be a
superset of each implementation's type, e.g. Fiber will have both an
`expirationTime` field and a `lanes` field. The implementations will
diverge, but not the types.
To do this, I lifted the type definitions into a separate module.
* Enable prefer-const rule
Stylistically I don't like this but Closure Compiler takes advantage of
this information.
* Auto-fix lints
* Manually fix the remaining callsites
This is equivalent to the jsx-runtime in that this is what the compiled
output on the server is supposed to target.
It's really just the same code for all the different Flights, but they
have different types in their arguments so each one gets their own entry
point. We might use this to add runtime warnings per entry point.
Unlike the client-side React.block call this doesn't provide the factory
function that curries the load function. The compiler is expected to wrap
this call in the currying factory.
* Formalize the Wakeable and Thenable types
We use two subsets of Promises throughout React APIs. This introduces
the smallest subset - Wakeable. It's the thing that you can throw to
suspend. It's something that can ping.
I also use a shared type for Thenable in the cases where we expect a value
so we can be a bit more rigid with our us of them.
* Make Chunks into Wakeables instead of using native Promises
This value is just going from here to React so we can keep it a lighter
abstraction throughout.
* Renamed thenable to wakeable in variable names
DevTools previously used the NPM events package for dispatching events. This package has an unfortunate flaw though- if a listener throws during event dispatch, no subsequent listeners are called. I've replaced that event dispatcher with my own implementation that ensures all listeners are called before it re-throws an error.
This commit replaces that event emitter with a custom implementation that calls all listeners before re-throwing an error.
* Resolve Server-side Blocks instead of Components
React elements should no longer be used to extract arbitrary data but only
for prerendering trees.
Blocks are used to create asynchronous behavior.
* Resolve Blocks in the Client
* Tests
* Bug fix relay JSON traversal
It's supposed to pass the original object and not the new one.
* Lint
* Move Noop Module Test Helpers to top level entry points
This module has shared state. It needs to be external from builds.
This lets us test the built versions of the Noop renderer.
Follow ups from https://github.com/facebook/react/pull/18334
I also introduced the concept of a module reference on the client too.
We don't need this for webpack so that gets compiled out but we need it
for www. Similarly I also need a difference between preload and load.
* Add ReactFlightServerConfig intermediate
This just forwards to the stream version of Flight which is itself forked
between Node and W3C streams.
The dom-relay goes directly to the Relay config though which allows it to
avoid the stream part of Flight.
* Separate streaming protocol into the Stream config
* Split streaming parts into the ReactFlightServerConfigStream
This decouples it so that the Relay implementation doesn't have to encode
the JSON to strings. Instead it can be fed the values as JSON objects and
do its own encoding.
* Split FlightClient into a basic part and a stream part
Same split as the server.
* Expose lower level async hooks to Relay
This requires an external helper file that we'll wire up internally.
* Rename to clarify that it's client-only
* Rename FizzStreamer to FizzServer for consistency
* Rename react-flight to react-client/flight
For consistency with react-server. Currently this just includes flight
but it could be expanded to include the whole reconciler.
* Add Relay Flight Build
* Rename ReactServerHostConfig to ReactServerStreamConfig
This will be the config specifically for streaming purposes.
There will be other configs for other purposes.
* Require deep for reconcilers
* Delete inline* files
* Delete react-reconciler/persistent
This no longer makes any sense because it react-reconciler takes
supportsMutation or supportsPersistence as options. It's no longer based
on feature flags.
* Fix jest mocking
* Fix Flow strategy
We now explicitly list which paths we want to be checked by a renderer.
For every other renderer config we ignore those paths.
Nothing is "any" typed. So if some transitive dependency isn't reachable
it won't be accidentally "any" that leaks.
In CI, we run our test suite against multiple build configurations. For
example, we run our tests in both dev and prod, and in both the
experimental and stable release channels. This is to prevent accidental
deviations in behavior between the different builds. If there's an
intentional deviation in behavior, the test author must account
for them.
However, we currently don't run tests against the www builds. That's
a problem, because it's common for features to land in www before they
land anywhere else, including the experimental release channel.
Typically we do this so we can gradually roll out the feature behind
a flag before deciding to enable it.
The way we test those features today is by mutating the
`shared/ReactFeatureFlags` module. There are a few downsides to this
approach, though. The flag is only overridden for the specific tests or
test suites where you apply the override. But usually what you want is
to run *all* tests with the flag enabled, to protect against unexpected
regressions.
Also, mutating the feature flags module only works when running the
tests against source, not against the final build artifacts, because the
ReactFeatureFlags module is inlined by the build script.
Instead, we should run the test suite against the www configuration,
just like we do for prod, experimental, and so on. I've added a new
command, `yarn test-www`. It automatically runs in CI.
Some of the www feature flags are dynamic; that is, they depend on
a runtime condition (i.e. a GK). These flags are imported from an
external module that lives in www. Those flags will be enabled for some
clients and disabled for others, so we should run the tests against
*both* modes.
So I've added a new global `__VARIANT__`, and a new test command `yarn
test-www-variant`. `__VARIANT__` is set to false by default; when
running `test-www-variant`, it's set to true.
If we were going for *really* comprehensive coverage, we would run the
tests against every possible configuration of feature flags: 2 ^
numberOfFlags total combinations. That's not practical, though, so
instead we only run against two combinations: once with `__VARIANT__`
set to `true`, and once with it set to `false`. We generally assume that
flags can be toggled independently, so in practice this should
be enough.
You can also refer to `__VARIANT__` in tests to detect which mode you're
running in. Or, you can import `shared/ReactFeatureFlags` and read the
specific flag you can about. However, we should stop mutating that
module going forward. Treat it as read-only.
In this commit, I have only setup the www tests to run against source.
I'll leave running against build for a follow up.
Many of our tests currently assume they run only in the default
configuration, and break when certain flags are toggled. Rather than fix
these all up front, I've hard-coded the relevant flags to the default
values. We can incrementally migrate those tests later.
