Sizebot works by fetching the base artifacts from CI. CircleCI recently
updated this endpoint to require an auth token. This is a problem for PR
branches, where sizebot runs, because we don't want to leak the token to
arbitrary code written by an outside contributor.
This only affects PR branches. CI workflows that run on the main branch
are allowed to access environment variables, because only those with
push access can land code in main.
As a temporary workaround, we'll fetch the assets from a mirror,
react-builds.vercel.app. This is the same app that hosts the sizebot
diff previews.
Need to figure out a longer term solution. Perhaps by converting sizebot
into a proper GitHub app.
The `start` convention is a CRA convention but nobody else of the modern
frameworks / tools use this convention for a file watcher and dev mode.
Instead the common convention is `dev`. Instead `start` is for running a
production build that's already been built.
---------
Co-authored-by: Sebastian Silbermann <silbermann.sebastian@gmail.com>
It's confusing to new contributors, and me, that you're supposed to use
`yarn build-combined` for almost everything but not fixtures.
We should use only one build command for everything.
Updated fixtures to use the folder convention of build-combined.
The current caching of steps of `node_modules` doesn't work reliable as
is because it includes `arch` in the cache key. `arch` might be
different across workers in the same commit.
I couldn't find a way to optionally restore caches, so what this PR does
is:
- remove the setup step that ran before all other steps and essentially
just populates a circle CI cache key
- all other steps now do: restore yarn cache, `yarn install`, save yarn
cache (fast if already exists)
With this change the initial batch of jobs all race to populate the
cache, but any subsequent jobs should find the existing cache. The
expected downside would be slightly more worker CPU time with all the
parallel jobs, but wall time might be shorter (1 step less in the
critical path) and we should be more reliable as we no longer have the
failure with multiple archs.
## Alternative 1
Remove the `{arch}` from the cache key.
Downside: this might run into weird issues with native dependencies.
## Alternative 2
Somehow check if the cache was restored and only then run a yarn
install.
Downside: couldn't figure out if it's possible to only restore the yarn
cache if restoring the node_modules cache failed. Without that we'd
either always restore both the yarn and node_modules cache or do yarn
installs w/o cache which are prone to failure in the past.
We've heard from multiple contributors that the Reconciler forking
mechanism was confusing and/or annoying to deal with. Since it's
currently unused and there's no immediate plans to start using it again,
this removes the forking.
Fully removing the fork is split into 2 steps to preserve file history:
**This PR**
- remove `enableNewReconciler` feature flag.
- remove `unstable_isNewReconciler` export
- remove eslint rules for cross fork imports
- remove `*.new.js` files and update imports
- merge non-suffixed files into `*.old` files where both exist
(sometimes types were defined there)
**#25775**
- rename `*.old` files
This PR adds a new GitHub action to commit build artifacts for Facebook
into a new protected branch. This will later be used to setup an
automatic sync to www.
The hacky spinloop is meant to be a temporary implementation until we
can support running a script internally on top of the synced diff
(coming soon). A GitHub token is otherwise required if we want to setup
a pub/sub between cirleci and github but it's not straightforward to
provision one for our org. So this workaround should do for now since we
won't keep it around for too long.
Example of it running and creating a commit on the `builds/facebook-www`
branch:
https://github.com/facebook/react/actions/runs/3516958576/jobs/5894251359
* Move Fizz inline instructions to unified module
Instead of a separate module per instruction, this exports all of them
from a unified module.
In the next step, I'll add a script to generate this new module.
* Add script to generate inline Fizz runtime
This adds a script to generate the inline Fizz runtime. Previously, the
runtime source was in an inline comment, and a compiled version of the
instructions were hardcoded as strings into the Fizz implementation,
where they are injected into the HTML stream.
I've moved the source for the instructions to a regular JavaScript
module. A script compiles the instructions with Closure, then generates
another module that exports the compiled instructions as strings.
Then the Fizz runtime imports the instructions from the
generated module.
To build the instructions, run:
yarn generate-inline-fizz-runtime
In the next step, I'll add a CI check to verify that the generated files
are up to date.
* Check in CI if generated Fizz runtime is in sync
The generated Fizz runtime is checked into source. In CI, we'll ensure
it stays in sync by running the script and confirming nothing changed.
- `~/.yarn/cache` is now restored from an hierarchical cache key, if no precise match is found, we fallback to less precise ones.
- The yarn install in `fixtures/dom` is also cached. Notably, is utilizes the cache from root, but stores into its more precise key.
- Steps running in root no longer have a `yarn install` and rely on the cache from the setup step.
- Retry `yarn install` once on failure.
We don't need to run DevTools regression tests once an hour, and also it makes getting the most recent react build or react devtools build really annoying, so run them once a day instead
We have a currently unreproducible flaky e2e test. This PR captures snapshots on e2e test failures so we can better debug flaky e2e tests that don't fail locally.
* Track revs that intentionaly fork the reconciler
When we fork the the "old" and "new" reconciler implementations, it can
be difficult to keep track of which commits introduced the delta
in behavior. This makes bisecting difficult if one of the changes
introduces a bug.
I've added a new file called `forked-revisions` that contains the list
of commits that intentionally forked the reconcilers.
In CI, we'll confirm that the reconcilers are identical except for the
changes in the listed revisions. This also ensures that the revisions
can be cleanly reverted.
* [TEST] Add trivial divergence between forks
This should fail CI. We'll see if the next commit fixes it.
* [TEST] Update list of forked revisions
This should fix CI
* Revert temporary fork
This reverts the temporary fork added in the previous commits that was
used to test CI.
* Update error message when CI fails
The download_build job needs to persist its artifacts to the workspace
so downstream jobs can access them.
Persist the same directories as the normal build job.
Fixes a mistake in #24676. The get_base_build job downloads artifacts to
`base-build` instead of `build`, so that sizebot can compare the two
directories. For most other jobs, though, we want it to produce the
same output as the normal build job.
When running the hourly DevTools testing workflow, we don't need to
build React from scratch each time; we can download its build artifacts,
like we do for sizebot and the release workflow.
* Allow aritfacts download even if CI is broken
Adds an option to the download script to disable the CI check and
continue downloading the artifacts even if CI is broken.
I often rely on this to debug broken build artifacts. I was thinking
the sizebot should also use this when downloading the base artifacts
from main, since for the purposes of size tracking, it really doesn't
matter whether the base commit is broken.
* Sizebot should work even if base rev is broken
Sizebot works by downloading the build artifacts for the base revision
and comparing the fize sizes, but the download script will fail if
the base revision has a failing CI job. This happens more often than it
should because of flaky cron jobs, but even when it does, we shouldn't
let it affect the sizebot — for the purposes of tracking sizes, it
doesn't really matter whether the base revision is broken.
This PR adds a `--replaceBuild` option to the script that downloads older React version builds. If this flag is true, we will replace the contents of the `build` folder with the contents of the `build-regression` folder and remove the `build-regression` folder after, which was the original behavior.
However, for e2e tests, we need both the original build (for DevTools) and the new build (for the React Apps), so we need both the `build` and the `build-regression` folders. Not adding the `--replaceBuild` option will do this.
This PR also modifies the circle CI config to reflect this change.
This PR adds an hourly chron job on Circle CI that runs regression tests on the most recent DevTools build for React v16.0, v16.5, v16.8 v17.0 and v18.0.
This reverts #24106.
There was a regression in CircleCI's artifacts API recently where you
could no longer access artifacts without an authorization token. This
broke our size reporting CI job because we can't use an authorization
token on external PRs without potentially leaking it. As a temporary
workaround, I changed the size reporting job to use a public mirror of
our build artifacts.
The CircleCI API has since been fixed to no longer require
authorization, so we can revert the workaround.
* Add authorization header to artifacts request
CircleCI's artifacts API was updated; it now errors unless you're
logged in. This affects any of our workflows that download
build artifacts.
To fix, I added an authorization header to the request.
* Update sizbot to pull artifacts from public mirror
We can't use the normal download-build script in sizebot because it
depends on the CircleCI artifacts API, which was recently changed to
require authorization. And we can't pass an authorization token
without possibly leaking it to the public, since we run sizebot on
PRs from external contributors. As a temporary workaround, this job
will pull the artifacts from a public mirror that I set up. But we
should find some other solution so we don't have to maintain
the mirror.
Checks that if one React package depends on another, the current
version satisfies the given dependency range.
That way we don't forget to bump dependencies when we release a
new version.
CI starting running Node 16, which breaks some of our tests because
the error message text for undefined property access has changed.
We should pin to Node 14 until we are able to update the messages.
* Output FIXME during build for unminified errors
The invariant Babel transform used to output a FIXME comment if it
could not find a matching error code. This could happen if there were
a configuration mistake that caused an unminified message to
slip through.
Linting the compiled bundles is the most reliable way to do it because
there's not a one-to-one mapping between source modules and bundles. For
example, the same source module may appear in multiple bundles, some
which are minified and others which aren't.
This updates the transform to output the same messages for Error calls.
The source lint rule is still useful for catching mistakes during
development, to prompt you to update the error codes map before pushing
the PR to CI.
* Don't run error transform in development
We used to run the error transform in both production and development,
because in development it was used to convert `invariant` calls into
throw statements.
Now that don't use `invariant` anymore, we only have to run the
transform for production builds.
* Add ! to FIXME comment so Closure doesn't strip it
Don't love this solution because Closure could change this heuristic,
or we could switch to a differnt compiler that doesn't support it. But
it works.
Could add a bundle that contains an unminified error solely for the
purpose of testing it, but that seems like overkill.
* Alternate extract-errors that scrapes artifacts
The build script outputs a special FIXME comment when it fails to minify
an error message. CI will detect these comments and fail the workflow.
The comments also include the expected error message. So I added an
alternate extract-errors that scrapes unminified messages from the
build artifacts and updates `codes.json`.
This is nice because it works on partial builds. And you can also run it
after the fact, instead of needing build all over again.
* Disable error minification in more bundles
Not worth it because the number of errors does not outweight the size
of the formatProdErrorMessage runtime.
* Run extract-errors script in CI
The lint_build job already checks for unminified errors, but the output
isn't super helpful.
Instead I've added a new job that runs the extract-errors script and
fails the build if `codes.json` changes. It also outputs the expected
diff so you can easily see which messages were missing from the map.
* Replace old extract-errors script with new one
Deletes the old extract-errors in favor of extract-errors2
There's a downstream workflow that runs the `print-warnings` command. We
can make it faster by scraping the warnings in CI and storing the
result as a build artifact.
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`.
Now that all the CI jobs have been migrated to the new build script,
we can start renaming the `build2` directory to `build`.
Since there are lots of scripts that reference `build2`, including
downstream scripts that live outside this repo, I'm going to keep
the `build2` directory around as a copy of `build`.
Then once all the references are updated, I will delete the copy.
* Move lint job to new, combined CI workflow
Moves the lint job our new, combined CI workflow.
After this, there is only one job remaining to be migrated. Then we
can delete the old workflow and build script.
* Remove "stable" CI workflow
This workflow is now empty so we can remove it
Moves the RELEASE_CHANNEL_stable_yarn_test_dom_fixtures job to our new,
combined CI workflow.
After this, there are only two jobs remaining to be migrated. Then we
can delete the old workflow and build script.
I noticed that `enableSuspenseLayoutEffectSemantics` is not fully
implemented in persistent mode. I believe this was an oversight
because we don't have a CI job that runs tests in persistent mode and
with experimental flags enabled.
This adds additional test configurations to the CI job so we don't miss
stuff like this again. It doesn't fix the failing tests — I'll address
that separately.
This does not mean that a release of 18.0 is imminent, only that the
main branch includes breaking changes.
Also updates the versioning scheme of the `@next` channel to include
the upcoming semver number, as well as the word "alpha" to indicate the
stability of the release.
- Before: 0.0.0-e0d9b28999
- After: 18.0.0-alpha-e0d9b28999
Uses the layout of the build artifact directory to infer the format
of a given file, and which lint rules to apply.
This has the effect of decoupling the lint build job from the existing
Rollup script, so that if we ever add additional post-processing, or
if we replace Rollup, it will still work.
But the immediate motivation is to replace the separate "stable" and
"experimental" lint-build jobs with a single combined job.
My personal workflow is to develop against the www-modern release
channel, with the variant flags enabled, because it encompasses the
largest set of features. Then I rely on CI to run the tests against
all the other configurations.
So in practice, I almost always run
```
yarn test -r=www-modern --variant TEST_FILE
```
instead of
```
yarn test TEST_FILE
```
So, I've updated the `yarn test` command to use those options
by default.