Merge main branch from upstream facebook/react (particularly for compatibility with ESLint 8.x)
#1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…argument is an object + Added unit tests (facebook#24554) formatWithStyles currently doesn't style the array argument if the first argument is an object. This PR fixes this and also adds unit tests.
…specified (facebook#24555) In DevTools tests, if the REACT_VERSION specified, we know this is a regression test (testing older React Versions). Because a lot of tests test the DevTools front end and we don't want to run them in the regression test scenario, we decided to only run tests that have the // @reactVersion pragma defined. Because if there are no tests specified, jest will fail, we also opt to use jest.skip to skip all the tests that we don't want to run for a specific React version istead. This PR makes this change.
Add `--reactVersion` argument. This argument is only used in DevTools. When this is specified, run only the tests that have the `// @reactVersion` pragma that satisfies the semver version range. Otherwise, run tests as normal
…acebook#23365) This PR: * During the passive effect complete phase for Offscreen, we add all the transitions that were added to the update queue in the render phase to the transitions set on the memoziedState. We also add the stateNode for the Offscreen component to the root pendingSuspenseBoundaries map if the suspense boundary has gone from shown to fallback. We remove the stateNode if the boundary goes from fallback to shown. * During the passive effect complete phase for the HostRoot, for each transition that was initiated during this commit, we add a pending transitionStart callback. We also add them to the transition set on the memoizedState for the HostRoot. If the root pendingSuspenseBoundaries is empty, we add a pending transitionComplete callback.
This reverts facebook#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.
This PR adds the reactVersion pragma to tests. Tests without the reactVersion pragma won't be run if the reactVersion pragma isn't specified. Tested each React version manually with the pragma to make sure the tests pass
I added a `--sourceMaps` option to our test command that enables inline source maps. I've kept it disabled by default, since it makes the tests run slower. But it's super useful when attaching to a debugger.
Make the error messages clearer when the API doesn't respond with 200.
Change storeStressTestSync to use inline snapshots instead of a snapshot file. We want to do this because some tests are gated and not called in regression tests, and if snapshot tests are not called when there is a corresponding .snap file, that test will fail. Arguably inline snapshots are a better pattern anyway, so enforcing this in DevTools tests IMO makes sense
…tions (facebook#24584) `string.replaceAll` doesn't exist in our CircleCI Docker environment. We also don't need it in this case because `semver.satisfies` allows for whitespace when specifying a range. This PR removes the unnecessary call.
…24541) * Log unexpected warnings when testing with ReactDOMServerIntegrationTestUtils * Add test Following facebook#9230 (comment) except that `foo={true}` renders an empty string. See facebook#9230 (comment) for rationale. * Match Preact behavior for boolean props on custom elements * Poke CircleCI
Some older React versions have different module import names and are missing certain features. This PR mocks modules that don't exist and maps modules in older versions to the ones that are required in tests. Specifically: * In React v16.5, scheduler is named schedule * In pre concurrent React, there is no act * Prior to React v18.0, react-dom/client doesn't exist * In DevTools, we expect to use scheduler/tracing-profiling instead of scheduler/tracing
This PR adds a script that downloads the specified react version from NPM (ie. react-dom, react, and react-test-renderer) and replaces the corresponding files in the build folder with the downloaded react files. The scheduler package, unlike react-dom, react, and react-test-renderer, is versioned differently, so we also need to specifically account for that in the script.
We need the regression config moduleNameMapper to come before the current moduleNameMapper so when it tries to map "/^react-dom\/([^/]+)$/ it doesn't get confused. The reason is because order in which the mappings are defined matters. Patterns are checked one by one until one fits, and the most specific rule should be listed first.
…book#24601) 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.
* Move hydration code out of normal Suspense path Shuffling some code around to make it easier to follow. The logic for updating a dehydrated Suspense boundary is significantly different from the logic for a client-rendered Suspense boundary. Most of it was already lifted out into a separate function; this moves the remaining hydration-specific logic out of updateSuspenseComponent and into updateDehydratedSuspenseComponent instead. No expected changes to program behavior. * Extract hydration logic in complete phase, too Same as previous step but for the complete phase. This is a separate commit to make bisecting easier in case something breaks. The logic is very subtle but mostly all I've done is extract it to another function.
…facebook#24618) * Test to assert that hydration errors of an inner suspended boundary are not retained by an unsuspended outer boundary * lints
This PR: * Increases test retry count to 2 so that flaky tests have more of a chance to pass * Ideally most e2e tests will run for all React versions (and ensure DevTools elegantly fails if React doesn't support its features). However, some features aren't supported in older React versions at all (ex. Profiling) Add runOnlyForReactRange function in these cases to skip tests that don't satisfy the correct React semver range * Fix should allow searching for component by name test, which was flaky because sometimes the Searchbox would be unfocused the second time we try to type in it * Edited test Should allow elements to be inspected to check that element inspect gracefully fails in older React versions * Updated config to add a config.use.url field and a config.use.react_version field, which change depending on the React Version (and whether it's specified)
This PR adds an e2e regression app to the react-devtools-shell package. This app: * Has an app.js and an appLegacy.js entrypoint because apps prior to React 18 need to use ReactDOM.render. These files will create and render multiple test apps (though they currently only render the List) * Moved the ListApp out of the e2e folder and into an e2e-apps folder so that both e2e and e2e-regression can use the same test apps * Creates a ListAppLegacy app because prior to React 16.8 hooks didn't exist. * Added a devtools file for the e2e-regression * Modifies the webpack config so that the e2e-regression React app can use different a different React version than DevTools
On the server we have a similar translation map from the file path that the loader uses to the refer to the original module and to the bundled module ID. The Flight server is optimized to emit the smallest format for the client. However during SSR, the same client component might go by a different module ID since it's a different bundle than the client bundle. This provides an option to add a translation map from client ID to SSR ID when reading the Flight stream. Ideally we should have a special SSR Flight Client that takes this option but for now we only have one Client for both.
* [Fizz] Improve text separator byte efficiency Previously text separators were inserted following any Text node in Fizz. This increases bytes sent when streaming and in some cases such as title elements these separators are not interpreted as comment nodes and leak into the visual aspects of a page as escaped text. The reason simple tracking on the last pushed type doesn't work is that Segments can be filled in asynchronously later and so you cannot know in a single pass whether the preceding content was a text node or not. This commit adds a concept of TextEmbedding which provides a best effort signal to Segments on whether they are embedded within text. This allows the later resolution of that Segment to add text separators when possibly necessary but avoid them when they are surely not. The current implementation can only "peek" head if the segment is a the Root Segment or a Suspense Boundary Segment. In these cases we know there is no trailing text embedding and we can eliminate the separator at the end of the segment if the last emitted element was Text. In normal Segments we cannot peek and thus have to assume there might be a trailing text embedding and we issue a separator defensively. This should be rare in practice as it is assumed most components that will cause segment creation will also emit some markup at the edges. * [Fizz] Improve separator efficiency when flushing delayed segments The method by which we get segment markup into the DOM differs depending on when the Segment resolves. If a Segment resolves before flushing begins for it's parent it will be emitted inline with the parent markup. In these cases separators may be necessary because they are how we clue the browser into breakup up text into distinct nodes that will later match up with what will be hydrated on the client. If a Segment resolves after flushing has happened a script will be used to patch up the DOM in the client. when this happens if there are any text nodes on the boundary of the patch they won't be "merged" and thus will continue to have distinct representation as Nodes in the DOM. Thus we can avoid doing any separators at the boundaries in these cases. After applying these changes the only time you will get text separators as follows * in between serial text nodes that emit at the same time - these are necessary and cannot be eliminated unless we stop relying on the browser to automatically parse the correct text nodes when processing this HTML * after a final text node in a non-boundary segment that resolves before it's parent has flushed - these are sometimes extraneous, like when the next emitted thing is a non-Text node. In all other cases text separators should be omitted which means the general byte efficiency of this approach should be pretty good
* use return from onError * export getSuspenseInstanceFallbackError * stringToChunk * return string from onError in downstream type signatures * 1 more type * support encoding errors in html stream and escape user input This commit adds another way to get errors to the suspense instance by encoding them as dataset properties of a template element at the head of the boundary. Previously if there was an error before the boundary flushed there was no way to stream the error to the client because there would never be a client render instruction. Additionally the error is sent in 3 parts 1) error hash - this is always sent (dev or prod) if one is provided 2) error message - Dev only 3) error component stack - Dev only, this now captures the stack at the point of error Another item addressed in this commit is the escaping of potentially unsafe data. all error components are escaped as test for browers when written into the html and as javascript strings when written into a client render instruction. * nits Co-authored-by: Marco Salazar <salazarm@fb.com>
…cript (facebook#24621) 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.
…book#24642) Modified the `run_devtools_e2e_tests` script so that you can pass in a React version. If you pass in a version, it will build the DevTools shell and run the e2e tests with that version.
Usually we complete workInProgress before yielding but if that's the currently suspended one, we don't yet complete it in case we can immediately unblock it. If we get interrupted, however, we must unwind it. Where as we usually assume that we've already completed it. This shows up when the current work in progress was a Context that pushed and then it suspends in its immediate children. If we don't unwind, it won't pop and so we get an imbalance.
…ues (facebook#25222) * Instead of reading from window in two separate places, do this in a single function * Add some type safety
This update range includes: - `types_first` ([blog](https://flow.org/en/docs/lang/types-first/), all exports need annotated types) is default. I disabled this for now to make that change incremental. - Generics that escape the scope they are defined in are an error. I fixed some with explicit type annotations and some are suppressed that I didn't easily figure out.
add more accurate end time for transitions and update host configs with `requestPostPaintCallback` function and move post paint logic to another module and use it in the work loop
* useMemoCache impl * test for multiple calls in a component (from custom hook) * Use array of arrays for multiple calls; use alternate/local as the backup * code cleanup * fix internal test * oops we do not support nullable property access * Simplify implementation, still have questions on some of the PR feedback though * Gate all code based on the feature flag * refactor to use updateQueue * address feedback * Try to eliminate size increase in prod bundle * update to retrigger ci
- `~/.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.
Show DevTools icons in Edge browser panel
This commit adds a new hook `useEvent` per the RFC [here](reactjs/rfcs#220), gated as experimental. Co-authored-by: Rick Hanlon <rickhanlonii@gmail.com> Co-authored-by: Rick Hanlon <rickhanlonii@fb.com> Co-authored-by: Lauren Tan <poteto@users.noreply.github.com>
… of throwing Promises (facebook#25260) * [Flight] Align Chunks with Thenable used with experimental_use Use the field names used by the Thenable data structure passed to use(). These are considered public in this model. This adds another field since we use a separate field name for "reason". * Implement Thenable Protocol on Chunks This doesn't just ping but resolves/rejects with the value. * Subclass Promises * Pass key through JSON parsing * Wait for preloadModules before resolving module chunks * Initialize lazy resolved values before reading the result * Block a model from initializing if its direct dependencies are pending If a module is blocked, then we can't complete initializing a model. However, we can still let it parse, and then fill in the missing pieces later. We need to block it from resolving until all dependencies have filled in which we can do with a ref count. * Treat blocked modules or models as a special status We currently loop over all chunks at the end to error them if they're still pending. We shouldn't do this if they're pending because they're blocked on an external resource like a module because the module might not resolve before the Flight connection closes and that's not an error. In an alternative solution I had a set that tracked pending chunks and removed one at a time. While the loop at the end is faster it's more work as we go. I figured the extra status might also help debugging. For modules we can probably assume no forward references, and the first async module we can just use the promise as the chunk. So we could probably get away with this only on models that are blocked by modules.
* [Flight] Move from suspensey readRoot() to use(thenable) * Update noop tests These are no longer sync so they need some more significant updating. Some of these tests are written in a non-idiomatic form too which is not great. * Update Relay tests I kept these as sync for now and just assume a sync Promise. * Updated the main tests * Gate tests * We need to cast through any because Thenable doesn't support unknown strings
* Fix acorn import I'm not sure how this ever worked. * Fix cache to wait for entries already added to the chunk cache * Modernize API
…5277) * reorganize react-dom internals to match react * refactor and make forks work for flow and internal imports * flew too close to the sun * typo
* [DevTools][BE] Read username using gh in release script * better regex & fix lint
Co-authored-by: Robert Balicki <robertbalicki@fb.com>
There's a global queue (`concurrentQueues` in the ReactFiberConcurrentUpdates module) that is cleared at the beginning of each render phase. However, in the case of an eager `setState` bailout where the state is updated to same value as the current one, we add the update to the queue without scheduling a render. So the render phase never removes it from the queue. This can lead to a memory leak if it happens repeatedly without any other updates. There's only one place where this ever happens, so the fix was pretty straightforward. Currently there's no great way to test this from a Jest test, so I confirmed locally by checking in an existing test whether the array gets reset. @sompylasar had an interesting suggestion for how to catch these in the future: in the development build (perhaps behind a flag), use a Babel plugin to instrument all module-level variables. Then periodically sweep to confirm if something has leaked. The logic is that if there's no React work scheduled, and a module-level variable points to an object, it very likely indicates a memory leak.
* suspense boundary error digest to Error instance and deprecate digest from errorInfo for onRecoverableError * fix closure escape
…-hooks-static-hooks-config
|
Hi @luketanner, thanks for opening this PR! Unfortunately, I was a bit irresponsible with my last changes to this fork, and so it turns out I had a commit that was not pushed to GitHub (as well as some "local only hacks" that I decided to push as well). In the end, I prefer to keep my changes rebased/cherry-picked on top of the upstream branch, so I've gone ahead and done that in a new branch here: facebook/react@main...grncdr:react:eslint-plugin-react-hooks-static-hooks-2022-edition This has now been published to npm as version |
|
@grncdr Sure thing :) Thanks again for your work! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@grncdr Thanks for all your work on enhancing the
exhaustive-depsrule, it's been invaluable! Unfortunately, the files are quite a bit out-of-date now, and it's no longer compatible with the latest version of ESLint (8.x) due to the new requirement of settingmeta.hasSuggestions.It really only requires a one-line fix (adding
hasSuggestions: trueinto the existingmetaproperty inExhaustiveDeps.js), but since Facebook has already made the required change in the upstream repo (facebook#22248) I figured it might be better to just pull their changes -- which has the added benefit of keeping this fork up to date (currently it's 1603 commits behind facebook:main).If you'd like to verify that I haven't tried to sneak in any additional changes, you can compare mine against facebook:main and the only changes should be the ones you originally made to
ExhaustiveDeps.js.Alternatively, if you'd prefer, I can close this PR and open a new one just making that one-line change directly (for compatibility with ESLint 8).