diff --git a/.circleci/config.yml b/.circleci/config.yml index 5804192ca451..559add55dbd6 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -7,17 +7,13 @@ aliases: - &environment TZ: /usr/share/zoneinfo/America/Los_Angeles - - &restore_yarn_cache + - &restore_node_modules restore_cache: name: Restore node_modules cache keys: - - v2-node-{{ arch }}-{{ .Branch }}-{{ checksum "yarn.lock" }} - - v2-node-{{ arch }}-{{ .Branch }}- - - v2-node-{{ arch }}- - - &run_yarn - run: - name: Install Packages - command: yarn --frozen-lockfile + - v2-node-{{ arch }}-{{ .Branch }}-{{ checksum "yarn.lock" }}-node-modules + + - &TEST_PARALLELISM 20 - &attach_workspace at: build @@ -28,8 +24,7 @@ aliases: steps: - checkout - attach_workspace: *attach_workspace - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: node ./scripts/rollup/consolidateBundleSizes.js - run: ./scripts/circleci/pack_and_store_artifact.sh - store_artifacts: @@ -57,13 +52,29 @@ jobs: - run: name: Nodejs Version command: node --version - - *restore_yarn_cache - - *run_yarn + - restore_cache: + name: Restore yarn cache + key: v2-node-{{ arch }}-{{ checksum "yarn.lock" }}-yarn + - run: + name: Install Packages + command: yarn --frozen-lockfile --cache-folder ~/.cache/yarn - save_cache: - name: Save node_modules cache - key: v2-node-{{ arch }}-{{ .Branch }}-{{ checksum "yarn.lock" }} + # Store the yarn cache globally for all lock files with this same + # checksum. This will speed up the setup job for all PRs where the + # lockfile is the same. + name: Save yarn cache for future installs + key: v2-node-{{ arch }}-{{ checksum "yarn.lock" }}-yarn paths: - ~/.cache/yarn + - save_cache: + # Store node_modules for all jobs in this workflow so that they don't + # need to each run a yarn install for each job. This will speed up + # all jobs run on this branch with the same lockfile. + name: Save node_modules cache + # This cache key is per branch, a yarn install in setup is required. + key: v2-node-{{ arch }}-{{ .Branch }}-{{ checksum "yarn.lock" }}-node-modules + paths: + - node_modules yarn_lint: docker: *docker @@ -71,8 +82,7 @@ jobs: steps: - checkout - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: node ./scripts/prettier/index - run: node ./scripts/tasks/eslint - run: ./scripts/circleci/check_license.sh @@ -85,138 +95,136 @@ jobs: steps: - checkout - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: node ./scripts/tasks/flow-ci RELEASE_CHANNEL_stable_yarn_test: docker: *docker environment: *environment + parallelism: *TEST_PARALLELISM steps: - checkout - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: yarn test --release-channel=stable --ci yarn_test: docker: *docker environment: *environment + parallelism: *TEST_PARALLELISM steps: - checkout - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: yarn test --ci RELEASE_CHANNEL_stable_yarn_test_www: docker: *docker environment: *environment + parallelism: *TEST_PARALLELISM steps: - checkout - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: yarn test --release-channel=www-classic --ci RELEASE_CHANNEL_stable_yarn_test_www_variant: docker: *docker environment: *environment + parallelism: *TEST_PARALLELISM steps: - checkout - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: yarn test --release-channel=www-classic --variant --ci RELEASE_CHANNEL_stable_yarn_test_prod_www: docker: *docker environment: *environment + parallelism: *TEST_PARALLELISM steps: - checkout - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: yarn test --release-channel=www-classic --prod --ci RELEASE_CHANNEL_stable_yarn_test_prod_www_variant: docker: *docker environment: *environment + parallelism: *TEST_PARALLELISM steps: - checkout - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: yarn test --release-channel=www-classic --prod --variant --ci yarn_test_www: docker: *docker environment: *environment + parallelism: *TEST_PARALLELISM steps: - checkout - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: yarn test --release-channel=www-modern --ci yarn_test_www_variant: docker: *docker environment: *environment + parallelism: *TEST_PARALLELISM steps: - checkout - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: yarn test --release-channel=www-modern --variant --ci yarn_test_prod_www: docker: *docker environment: *environment + parallelism: *TEST_PARALLELISM steps: - checkout - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: yarn test --release-channel=www-modern --prod --ci yarn_test_prod_www_variant: docker: *docker environment: *environment + parallelism: *TEST_PARALLELISM steps: - checkout - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: yarn test --release-channel=www-modern --prod --variant --ci RELEASE_CHANNEL_stable_yarn_test_persistent: docker: *docker environment: *environment + parallelism: *TEST_PARALLELISM steps: - checkout - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: yarn test --release-channel=stable --persistent --ci RELEASE_CHANNEL_stable_yarn_test_prod: docker: *docker environment: *environment + parallelism: *TEST_PARALLELISM steps: - checkout - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: yarn test --release-channel=stable --prod --ci yarn_test_prod: docker: *docker environment: *environment + parallelism: *TEST_PARALLELISM steps: - checkout - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: yarn test --release-channel=experimental --prod --ci RELEASE_CHANNEL_stable_yarn_build: docker: *docker environment: *environment - parallelism: 20 + parallelism: *TEST_PARALLELISM steps: - checkout - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: environment: RELEASE_CHANNEL: stable @@ -242,8 +250,7 @@ jobs: parallelism: 20 steps: - checkout - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: environment: RELEASE_CHANNEL: experimental @@ -270,8 +277,7 @@ jobs: steps: - checkout - attach_workspace: *attach_workspace - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: environment: RELEASE_CHANNEL: experimental @@ -290,8 +296,7 @@ jobs: steps: - checkout - attach_workspace: *attach_workspace - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules # This runs in the process_artifacts job, too, but it's faster to run # this step in both jobs instead of running the jobs sequentially - run: node ./scripts/rollup/consolidateBundleSizes.js @@ -306,8 +311,7 @@ jobs: steps: - checkout - attach_workspace: *attach_workspace - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules # This runs in the process_artifacts job, too, but it's faster to run # this step in both jobs instead of running the jobs sequentially - run: node ./scripts/rollup/consolidateBundleSizes.js @@ -322,8 +326,7 @@ jobs: steps: - checkout - attach_workspace: *attach_workspace - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: yarn lint-build - run: scripts/circleci/check_minified_errors.sh @@ -333,8 +336,7 @@ jobs: steps: - checkout - attach_workspace: *attach_workspace - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: environment: RELEASE_CHANNEL: stable @@ -344,21 +346,21 @@ jobs: RELEASE_CHANNEL_stable_yarn_test_build: docker: *docker environment: *environment + parallelism: *TEST_PARALLELISM steps: - checkout - attach_workspace: *attach_workspace - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: yarn test --release-channel=stable --build --ci yarn_test_build: docker: *docker environment: *environment + parallelism: *TEST_PARALLELISM steps: - checkout - attach_workspace: *attach_workspace - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: yarn test --release-channel=experimental --build --ci yarn_test_build_devtools: @@ -367,8 +369,7 @@ jobs: steps: - checkout - attach_workspace: *attach_workspace - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: yarn test --project=devtools --build --ci RELEASE_CHANNEL_stable_yarn_test_dom_fixtures: @@ -377,7 +378,7 @@ jobs: steps: - checkout - attach_workspace: *attach_workspace - - *restore_yarn_cache + - *restore_node_modules - run: name: Run DOM fixture tests environment: @@ -393,8 +394,7 @@ jobs: environment: *environment steps: - checkout - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: name: Run fuzz tests command: | @@ -404,21 +404,21 @@ jobs: RELEASE_CHANNEL_stable_yarn_test_build_prod: docker: *docker environment: *environment + parallelism: *TEST_PARALLELISM steps: - checkout - attach_workspace: *attach_workspace - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: yarn test --release-channel=stable --build --prod --ci yarn_test_build_prod: docker: *docker environment: *environment + parallelism: *TEST_PARALLELISM steps: - checkout - attach_workspace: *attach_workspace - - *restore_yarn_cache - - *run_yarn + - *restore_node_modules - run: yarn test --release-channel=experimental --build --prod --ci workflows: diff --git a/.eslintrc.js b/.eslintrc.js index 179141ca82e1..6998bca453f1 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -166,6 +166,12 @@ module.exports = { __webpack_require__: true, }, }, + { + files: ['packages/scheduler/**/*.js'], + globals: { + TaskController: true, + }, + }, ], globals: { diff --git a/fixtures/nesting/.env b/fixtures/nesting/.env new file mode 100644 index 000000000000..eff23057e140 --- /dev/null +++ b/fixtures/nesting/.env @@ -0,0 +1,2 @@ +EXTEND_ESLINT=true +SKIP_PREFLIGHT_CHECK=true diff --git a/fixtures/nesting/.eslintignore b/fixtures/nesting/.eslintignore new file mode 100644 index 000000000000..4c368bf0fb57 --- /dev/null +++ b/fixtures/nesting/.eslintignore @@ -0,0 +1 @@ +src/*/node_modules diff --git a/fixtures/nesting/.gitignore b/fixtures/nesting/.gitignore new file mode 100644 index 000000000000..abaccc497db8 --- /dev/null +++ b/fixtures/nesting/.gitignore @@ -0,0 +1,27 @@ +# See https://help.github.com/articles/ignoring-files/ for more about ignoring files. + +# dependencies +/node_modules +/.pnp +.pnp.js + +# testing +/coverage + +# production +/build + +# misc +.DS_Store +.env.local +.env.development.local +.env.test.local +.env.production.local + +npm-debug.log* +yarn-debug.log* +yarn-error.log* + +# copies of shared +src/*/shared +src/*/node_modules diff --git a/fixtures/nesting/README.md b/fixtures/nesting/README.md new file mode 100644 index 000000000000..ac57b3a6347d --- /dev/null +++ b/fixtures/nesting/README.md @@ -0,0 +1,163 @@ +# Nested React Demo + +This is a demo of how you can configure a build system to serve **two different versions of React** side by side in the same app. This is not optimal, and should only be used as a compromise to prevent your app from getting stuck on an old version of React. + +## You Probably Don't Need This + +Note that **this approach is meant to be an escape hatch, not the norm**. + +Normally, we encourage you to use a single version of React across your whole app. When you need to upgrade React, it is better to try to upgrade it all at once. We try to keep breaking changes between versions to the minimum, and often there are automatic scripts ("codemods") that can assist you with migration. You can always find the migration information for any release on [our blog](https://reactjs.org/blog/). + +Using a single version of React removes a lot of complexity. It is also essential to ensure the best experience for your users who don't have to download the code twice. Always prefer using one React. + +## What Is This For? + +However, for some apps that have been in production for many years, upgrading all screens at once may be prohibitively difficult. For example, React components written in 2014 may still rely on [the unofficial legacy context API](https://reactjs.org/docs/legacy-context.html) (not to be confused with the modern one), and are not always maintained. + +Traditionally, this meant that if a legacy API is deprecated, you would be stuck on the old version of React forever. That prevents your whole app from receiving improvements and bugfixes. This repository demonstrates a hybrid approach. It shows how you can use a newer version of React for some parts of your app, while **lazy-loading an older version of React** for the parts that haven't been migrated yet. + +This approach is inherently more complex, and should be used as a last resort when you can't upgrade. + +## Version Requirements + +This demo uses two different versions of React: React 17 for "modern" components (in `src/modern`), and React 16.8 for "legacy" components (in `src/legacy`). + +**We still recommend upgrading your whole app to React 17 in one piece.** The React 17 release intentionally has minimal breaking changes so that it's easier to upgrade to. In particular, React 17 solves some problems with nesting related to event propagation that earlier versions of React did not handle well. We expect that this nesting demo may not be as useful today as during a future migration from React 17 to the future major versions where some of the long-deprecated APIs may be removed. + +However, if you're already stuck on an old version of React, you may found this approach useful today. If you remove a Hook call from `src/shared/Clock.js`, you can downgrade the legacy React all the way down to React 16.3. If you then remove Context API usage from `src/legacy/createLegacyRoot.js`, you can further downgrade the legacy React version, but keep in mind that the usage of third-party libraries included in this demo (React Router and React Redux) may need to be adjusted or removed. + +## Installation + +To run this demo, open its folder in Terminal and execute: + +```sh +npm install +npm start +``` + +If you want to test the production build, you can run instead: + +``` +npm install +npm run build +npx serve -s build +``` + +This sample app uses client-side routing and consists of two routes: + +- `/` renders a page which uses a newer version of React. (In the production build, you can verify that only one version of React is being loaded when this route is rendered.) +- `/about` renders a page which uses an older version of React for a part of its tree. (In the production build, you can verify that both versions of React are loaded from different chunks.) + +**The purpose of this demo is to show some nuances of such setup:** + +- How to install two versions of React in a single app with npm side by side. +- How to avoid the ["invalid Hook call" error](https://github.com/facebook/react/issues/13991) while nesting React trees. +- How to pass context between different versions of React. +- How to lazy-load the second React bundle so it's only loaded on the screens that use it. +- How to do all of this without a special bundler configuration. + +## How It Works + +File structure is extremely important in this demo. It has a direct effect on which code is going to use which version of React. This particular demo is using Create React App without ejecting, so **it doesn't rely on any bundler plugins or configuration**. The principle of this demo is portable to other setups. + +### Dependencies + +We will use three different `package.json`s: one for non-React code at the root, and two in the respective `src/legacy` and `src/modern` folders that specify the React dependencies: + +- **`package.json`**: The root `package.json` is a place for build dependencies (such as `react-scripts`) and any React-agnostic libraries (for example, `lodash`, `immer`, or `redux`). We do **not** include React or any React-related libraries in this file. +- **`src/legacy/package.json`**: This is where we declare the `react` and `react-dom` dependencies for the "legacy" trees. In this demo, we're using React 16.8 (although, as noted above, we could downgrade it further below). This is **also** where we specify any third-party libraries that use React. For example, we include `react-router` and `react-redux` in this example. +- **`src/modern/package.json`**: This is where we declare the `react` and `react-dom` dependencies for the "modern" trees. In this demo, we're using React 17. Here, we also specify third-party dependencies that use React and are used from the modern part of our app. This is why we *also* have `react-router` and `react-redux` in this file. (Their versions don't strictly have to match their `legacy` counterparts, but features that rely on context may require workarounds if they differ.) + +The `scripts` in the root `package.json` are set up so that when you run `npm install` in it, it also runs `npm intall` in both `src/legacy` and `src/modern` folders. + +**Note:** This demo is set up to use a few third-party dependencies (React Router and Redux). These are not essential, and you can remove them from the demo. They are included so we can show how to make them work with this approach. + +### Folders + +There are a few key folders in this example: + +- **`src`**: Root of the source tree. At this level (or below it, except for the special folders noted below), you can put any logic that's agnostic of React. For example, in this demo we have `src/index.js` which is the app's entry point, and `src/store.js` which exports a Redux store. These regular modules only execute once, and are **not** duplicated between the bundles. +- **`src/legacy`**: This is where all the code using the older version of React should go. This includes React components and Hooks, and general product code that is **only** used by the legacy trees. +- **`src/modern`**: This is where all the code using the newer version of React should go. This includes React components and Hooks, and general product code that is **only** used by the modern trees. +- **`src/shared`**: You may have some components or Hooks that you wish to use from both modern and legacy subtrees. The build process is set up so that **everything inside `src/shared` gets copied by a file watcher** into both `src/legacy/shared` and `src/modern/shared` on every change. This lets you write a component or a Hook once, but reuse it in both places. + +### Lazy Loading + +Loading two Reacts on the same page is bad for the user experience, so you should strive to push this as far as possible from the critical path of your app. For example, if there is a dialog that is less commonly used, or a route that is rarely visited, those are better candidates for staying on an older version of React than parts of your homepage. + +To encourage only loading the older React when necessary, this demo includes a helper that works similarly to `React.lazy`. For example, `src/modern/AboutPage.js`, simplified, looks like this: + +```js +import lazyLegacyRoot from './lazyLegacyRoot'; + +// Lazy-load a component from the bundle using legacy React. +const Greeting = lazyLegacyRoot(() => import('../legacy/Greeting')); + +function AboutPage() { + return ( + <> +

This component is rendered by React ({React.version}).

+ + + ); +} +``` + +As a result, only if the `AboutPage` (and as a result, ``) gets rendered, we will load the bundle containing the legacy React and the legacy `Greeting` component. Like with `React.lazy()`, we wrap it in `` to specify the loading indicator: + +```js +}> + + +``` + +If the legacy component is only rendered conditionally, we won't load the second React until it's shown: + +```js +<> + + {showGreeting && ( + }> + + + )} + +``` + + +The implementation of the `src/modern/lazyLegacyRoot.js` helper is included so you can tweak it and customize it to your needs. Remember to test lazy loading with the production builds because the bundler may not optimize it in development. + +### Context + +If you have nested trees managed by different versions of React, the inner tree won't "see" the outer tree's Context. + +This breaks third-party libraries like React Redux or React Router, as well as any of your own usage of Context (for example, for theming). + +To solve this problem, we read all the Contexts we care about in the outer tree, pass them to the inner tree, and then wrap the inner tree in the corresponding Providers. You can see this in action in two files: + +* `src/modern/lazyLegacyRoot.js`: Look for `useContext` calls, and how their results are combined into a single object that is passed through. **You can read more Contexts there** if your app requires them. +* `src/legacy/createLegacyRoot.js`: Look for the `Bridge` component which receives that object and wraps its children with the appropriate Context Providers. **You can wrap them with more Providers there** if your app requires them. + +Note that, generally saying, this approach is somewhat fragile, especially because some libraries may not expose their Contexts officially or consider their structure private. You may be able to expose private Contexts by using a tool like [patch-package](https://www.npmjs.com/package/patch-package), but remember to keep all the versions pinned because even a patch release of a third-party library may change the behavior. + +### Nesting Direction + +In this demo, we use an older React inside an app managed by the newer React. However, we could rename the folders and apply the same approach in the other direction. + +### Event Propagation + +Note that before React 17, `event.stopPropagation()` in the inner React tree does not prevent the event propagation to the outer React tree. This may cause unexpected behavior when extracting a UI tree like a dialog to use a separate React. This is because prior to React 17, both Reacts would attach the event listener at the `document` level. React 17 fixes this by attaching handlers to the roots. We strongly recommend upgrading to React 17 before considering the nesting strategy for future upgrades. + +### Gotchas + +This setup is unusual, so it has a few gotchas. + +* Don't add `package.json` to the `src/shared` folder. For example, if you want to use an npm React component inside `src/shared`, you should add it to both `src/modern/package.json` and `src/legacy/package.json` instead. You can use different versions of it but make sure your code works with both of them — and that it works with both Reacts! +* Don't use React outside of the `src/modern`, `src/legacy`, or `src/shared`. Don't add React-related libraries outside of `src/modern/package.json` or `src/legacy/package.json`. +* Remember that `src/shared` is where you write shared components, but the files you write there are automatically copied into `src/modern/shared` and `src/legacy/shared`, **from which you should import them**. Both of the target directories are in `.gitignore`. Importing directly from `src/shared` **will not work** because it is ambiguous what `react` refers to in that folder. +* Keep in mind that any code in `src/shared` gets duplicated between the legacy and the modern bundles. Code that should not be duplicated needs to be anywhere else in `src` (but you can't use React there since the version is ambiguous). +* You'll want to exclude `src/*/node_modules` from your linter's configuration, as this demo does in `.eslintignorerc`. + +This setup is complicated, and we don't recommend it for most apps. However, we believe it is important to offer it as an option for apps that would otherwise get left behind. There might be ways to simplify it with a layer of tooling, but this example is intentionally showing the low-level mechanism that other tools may build on. diff --git a/fixtures/nesting/package.json b/fixtures/nesting/package.json new file mode 100644 index 000000000000..130ecfa54e77 --- /dev/null +++ b/fixtures/nesting/package.json @@ -0,0 +1,43 @@ +{ + "name": "react-nesting-example", + "version": "0.1.0", + "private": true, + "dependencies": { + "react-scripts": "3.4.1", + "redux": "^4.0.5" + }, + "scripts": { + "postinstall": "run-p install:*", + "install:legacy": "cd src/legacy && npm install", + "install:modern": "cd src/modern && npm install", + "copy:legacy": "cpx 'src/shared/**' 'src/legacy/shared/'", + "copy:modern": "cpx 'src/shared/**' 'src/modern/shared/'", + "watch:legacy": "cpx 'src/shared/**' 'src/legacy/shared/' --watch --no-initial", + "watch:modern": "cpx 'src/shared/**' 'src/modern/shared/' --watch --no-initial", + "prebuild": "run-p copy:*", + "prestart": "run-p copy:*", + "start": "run-p start-app watch:*", + "start-app": "react-scripts start", + "build": "react-scripts build", + "eject": "react-scripts eject" + }, + "eslintConfig": { + "extends": "react-app" + }, + "browserslist": { + "production": [ + ">0.2%", + "not dead", + "not op_mini all" + ], + "development": [ + "last 1 chrome version", + "last 1 firefox version", + "last 1 safari version" + ] + }, + "devDependencies": { + "cpx": "^1.5.0", + "npm-run-all": "^4.1.5" + } +} diff --git a/fixtures/nesting/public/index.html b/fixtures/nesting/public/index.html new file mode 100644 index 000000000000..6252f08e4954 --- /dev/null +++ b/fixtures/nesting/public/index.html @@ -0,0 +1,12 @@ + + + + + + React App + + + +
+ + diff --git a/fixtures/nesting/src/index.js b/fixtures/nesting/src/index.js new file mode 100644 index 000000000000..6a70a3242442 --- /dev/null +++ b/fixtures/nesting/src/index.js @@ -0,0 +1 @@ +import './modern/index'; diff --git a/fixtures/nesting/src/legacy/Greeting.js b/fixtures/nesting/src/legacy/Greeting.js new file mode 100644 index 000000000000..11bf3faf31c4 --- /dev/null +++ b/fixtures/nesting/src/legacy/Greeting.js @@ -0,0 +1,51 @@ +import React from 'react'; +import {Component} from 'react'; +import {findDOMNode} from 'react-dom'; +import {Link} from 'react-router-dom'; +import {connect} from 'react-redux'; +import {store} from '../store'; + +import ThemeContext from './shared/ThemeContext'; +import Clock from './shared/Clock'; + +store.subscribe(() => { + console.log('Counter:', store.getState()); +}); + +class AboutSection extends Component { + componentDidMount() { + // The modern app is wrapped in StrictMode, + // but the legacy bits can still use old APIs. + findDOMNode(this); + } + render() { + return ( + + {theme => ( +
+

src/legacy/Greeting.js

+

+ This component is rendered by the nested React ({React.version}). +

+ +

+ Counter: {this.props.counter}{' '} + +

+ + Go to Home + +
+ )} + + ); + } +} + +function mapStateToProps(state) { + return {counter: state}; +} + +export default connect(mapStateToProps)(AboutSection); diff --git a/fixtures/nesting/src/legacy/createLegacyRoot.js b/fixtures/nesting/src/legacy/createLegacyRoot.js new file mode 100644 index 000000000000..ec4941b5d929 --- /dev/null +++ b/fixtures/nesting/src/legacy/createLegacyRoot.js @@ -0,0 +1,43 @@ +/* eslint-disable react/jsx-pascal-case */ + +import React from 'react'; +import ReactDOM from 'react-dom'; +import ThemeContext from './shared/ThemeContext'; + +// Note: this is a semi-private API, but it's ok to use it +// if we never inspect the values, and only pass them through. +import {__RouterContext} from 'react-router'; +import {Provider} from 'react-redux'; + +// Pass through every context required by this tree. +// The context object is populated in src/modern/withLegacyRoot. +function Bridge({children, context}) { + return ( + + <__RouterContext.Provider value={context.router}> + {/* + If we used the newer react-redux@7.x in the legacy/package.json, + we woud instead import {ReactReduxContext} from 'react-redux' + and render . + */} + {children} + + + ); +} + +export default function createLegacyRoot(container) { + return { + render(Component, props, context) { + ReactDOM.render( + + + , + container + ); + }, + unmount() { + ReactDOM.unmountComponentAtNode(container); + }, + }; +} diff --git a/fixtures/nesting/src/legacy/package.json b/fixtures/nesting/src/legacy/package.json new file mode 100644 index 000000000000..7517005ef9ec --- /dev/null +++ b/fixtures/nesting/src/legacy/package.json @@ -0,0 +1,10 @@ +{ + "private": true, + "name": "react-nesting-example-legacy", + "dependencies": { + "react": "16.8", + "react-dom": "16.8", + "react-redux": "4.4.10", + "react-router-dom": "5.2.0" + } +} diff --git a/fixtures/nesting/src/modern/AboutPage.js b/fixtures/nesting/src/modern/AboutPage.js new file mode 100644 index 000000000000..27e52dad5a83 --- /dev/null +++ b/fixtures/nesting/src/modern/AboutPage.js @@ -0,0 +1,33 @@ +import React from 'react'; +import {useContext} from 'react'; +import {connect} from 'react-redux'; + +import ThemeContext from './shared/ThemeContext'; +import lazyLegacyRoot from './lazyLegacyRoot'; + +// Lazy-load a component from the bundle using legacy React. +const Greeting = lazyLegacyRoot(() => import('../legacy/Greeting')); + +function AboutPage({counter, dispatch}) { + const theme = useContext(ThemeContext); + return ( + <> +

src/modern/AboutPage.js

+

+ This component is rendered by the outer React ({React.version}). +

+ +
+

+ Counter: {counter}{' '} + +

+ + ); +} + +function mapStateToProps(state) { + return {counter: state}; +} + +export default connect(mapStateToProps)(AboutPage); diff --git a/fixtures/nesting/src/modern/App.js b/fixtures/nesting/src/modern/App.js new file mode 100644 index 000000000000..51782179284c --- /dev/null +++ b/fixtures/nesting/src/modern/App.js @@ -0,0 +1,52 @@ +import React from 'react'; +import {useState, Suspense} from 'react'; +import {BrowserRouter, Switch, Route} from 'react-router-dom'; + +import HomePage from './HomePage'; +import AboutPage from './AboutPage'; +import ThemeContext from './shared/ThemeContext'; + +export default function App() { + const [theme, setTheme] = useState('slategrey'); + + function handleToggleClick() { + if (theme === 'slategrey') { + setTheme('hotpink'); + } else { + setTheme('slategrey'); + } + } + + return ( + + +
+
+ +
+ }> + + + + + + + + + +
+
+ + + ); +} + +function Spinner() { + return null; +} diff --git a/fixtures/nesting/src/modern/HomePage.js b/fixtures/nesting/src/modern/HomePage.js new file mode 100644 index 000000000000..7b23652ef5c1 --- /dev/null +++ b/fixtures/nesting/src/modern/HomePage.js @@ -0,0 +1,22 @@ +import React from 'react'; +import {useContext} from 'react'; +import {Link} from 'react-router-dom'; + +import ThemeContext from './shared/ThemeContext'; +import Clock from './shared/Clock'; + +export default function HomePage({counter, dispatch}) { + const theme = useContext(ThemeContext); + return ( + <> +

src/modern/HomePage.js

+

+ This component is rendered by the outer React ({React.version}). +

+ + + Go to About + + + ); +} diff --git a/fixtures/nesting/src/modern/index.js b/fixtures/nesting/src/modern/index.js new file mode 100644 index 000000000000..940886e9bbaf --- /dev/null +++ b/fixtures/nesting/src/modern/index.js @@ -0,0 +1,15 @@ +import React from 'react'; +import {StrictMode} from 'react'; +import ReactDOM from 'react-dom'; +import {Provider} from 'react-redux'; +import App from './App'; +import {store} from '../store'; + +ReactDOM.render( + + + + + , + document.getElementById('root') +); diff --git a/fixtures/nesting/src/modern/lazyLegacyRoot.js b/fixtures/nesting/src/modern/lazyLegacyRoot.js new file mode 100644 index 000000000000..648b2aea3fe2 --- /dev/null +++ b/fixtures/nesting/src/modern/lazyLegacyRoot.js @@ -0,0 +1,94 @@ +import React from 'react'; +import {useContext, useMemo, useRef, useState, useLayoutEffect} from 'react'; +import {__RouterContext} from 'react-router'; +import {ReactReduxContext} from 'react-redux'; + +import ThemeContext from './shared/ThemeContext'; + +let rendererModule = { + status: 'pending', + promise: null, + result: null, +}; + +export default function lazyLegacyRoot(getLegacyComponent) { + let componentModule = { + status: 'pending', + promise: null, + result: null, + }; + + return function Wrapper(props) { + const createLegacyRoot = readModule(rendererModule, () => + import('../legacy/createLegacyRoot') + ).default; + const Component = readModule(componentModule, getLegacyComponent).default; + const containerRef = useRef(null); + const rootRef = useRef(null); + + // Populate every contexts we want the legacy subtree to see. + // Then in src/legacy/createLegacyRoot we will apply them. + const theme = useContext(ThemeContext); + const router = useContext(__RouterContext); + const reactRedux = useContext(ReactReduxContext); + const context = useMemo( + () => ({ + theme, + router, + reactRedux, + }), + [theme, router, reactRedux] + ); + + // Create/unmount. + useLayoutEffect(() => { + if (!rootRef.current) { + rootRef.current = createLegacyRoot(containerRef.current); + } + const root = rootRef.current; + return () => { + root.unmount(); + }; + }, [createLegacyRoot]); + + // Mount/update. + useLayoutEffect(() => { + if (rootRef.current) { + rootRef.current.render(Component, props, context); + } + }, [Component, props, context]); + + return
; + }; +} + +// This is similar to React.lazy, but implemented manually. +// We use this to Suspend rendering of this component until +// we fetch the component and the legacy React to render it. +function readModule(record, createPromise) { + if (record.status === 'fulfilled') { + return record.result; + } + if (record.status === 'rejected') { + throw record.result; + } + if (!record.promise) { + record.promise = createPromise().then( + value => { + if (record.status === 'pending') { + record.status = 'fulfilled'; + record.promise = null; + record.result = value; + } + }, + error => { + if (record.status === 'pending') { + record.status = 'rejected'; + record.promise = null; + record.result = error; + } + } + ); + } + throw record.promise; +} diff --git a/fixtures/nesting/src/modern/package.json b/fixtures/nesting/src/modern/package.json new file mode 100644 index 000000000000..4d11e94913de --- /dev/null +++ b/fixtures/nesting/src/modern/package.json @@ -0,0 +1,10 @@ +{ + "private": true, + "name": "react-nesting-example-modern", + "dependencies": { + "react": "0.0.0-3d0895557", + "react-dom": "0.0.0-3d0895557", + "react-redux": "7.2.1", + "react-router-dom": "5.2.0" + } +} diff --git a/fixtures/nesting/src/shared/Clock.js b/fixtures/nesting/src/shared/Clock.js new file mode 100644 index 000000000000..8c4b36696ba1 --- /dev/null +++ b/fixtures/nesting/src/shared/Clock.js @@ -0,0 +1,8 @@ +import React from 'react'; + +import useTime from './useTime'; + +export default function Clock() { + const time = useTime(); + return

Time: {time}

; +} diff --git a/fixtures/nesting/src/shared/ThemeContext.js b/fixtures/nesting/src/shared/ThemeContext.js new file mode 100644 index 000000000000..caa4b07322ac --- /dev/null +++ b/fixtures/nesting/src/shared/ThemeContext.js @@ -0,0 +1,5 @@ +import {createContext} from 'react'; + +const ThemeContext = createContext(null); + +export default ThemeContext; diff --git a/fixtures/nesting/src/shared/useTime.js b/fixtures/nesting/src/shared/useTime.js new file mode 100644 index 000000000000..afe7bbf4580d --- /dev/null +++ b/fixtures/nesting/src/shared/useTime.js @@ -0,0 +1,12 @@ +import {useState, useEffect} from 'react'; + +export default function useTimer() { + const [value, setValue] = useState(() => new Date()); + useEffect(() => { + const id = setInterval(() => { + setValue(new Date()); + }, 1000); + return () => clearInterval(id); + }, []); + return value.toLocaleTimeString(); +} diff --git a/fixtures/nesting/src/store.js b/fixtures/nesting/src/store.js new file mode 100644 index 000000000000..d9031980ec9f --- /dev/null +++ b/fixtures/nesting/src/store.js @@ -0,0 +1,14 @@ +import {createStore} from 'redux'; + +function reducer(state = 0, action) { + switch (action.type) { + case 'increment': + return state + 1; + default: + return state; + } +} + +// Because this file is declared above both Modern and Legacy folders, +// we can import this from either folder without duplicating the object. +export const store = createStore(reducer); diff --git a/packages/eslint-plugin-react-hooks/CHANGELOG.md b/packages/eslint-plugin-react-hooks/CHANGELOG.md index c173ac08f9a0..c06ad9eabd02 100644 --- a/packages/eslint-plugin-react-hooks/CHANGELOG.md +++ b/packages/eslint-plugin-react-hooks/CHANGELOG.md @@ -1,3 +1,6 @@ +## 4.1.0 +* **New Violations:** Warn when dependencies change on every render. ([@captbaritone](https://github.com/captbaritone) in [#19590](https://github.com/facebook/react/pull/19590)) + ## 4.0.8 * Fixes TypeScript `typeof` annotation to not be considered a dependency. ([@delca85](https://github.com/delca85) in [#19316](https://github.com/facebook/react/pull/19316)) diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js index de6d0aa4f7e0..4de51e9c4043 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js @@ -56,7 +56,7 @@ const tests = { { code: normalizeIndent` function MyComponent() { - const local = {}; + const local = someFunc(); useEffect(() => { console.log(local); }, [local]); @@ -94,9 +94,9 @@ const tests = { { code: normalizeIndent` function MyComponent() { - const local1 = {}; + const local1 = someFunc(); { - const local2 = {}; + const local2 = someFunc(); useCallback(() => { console.log(local1); console.log(local2); @@ -108,9 +108,9 @@ const tests = { { code: normalizeIndent` function MyComponent() { - const local1 = {}; + const local1 = someFunc(); function MyNestedComponent() { - const local2 = {}; + const local2 = someFunc(); useCallback(() => { console.log(local1); console.log(local2); @@ -122,7 +122,7 @@ const tests = { { code: normalizeIndent` function MyComponent() { - const local = {}; + const local = someFunc(); useEffect(() => { console.log(local); console.log(local); @@ -142,7 +142,7 @@ const tests = { { code: normalizeIndent` function MyComponent() { - const local = {}; + const local = someFunc(); useEffect(() => { console.log(local); }, [,,,local,,,]); @@ -222,7 +222,7 @@ const tests = { { code: normalizeIndent` function MyComponent(props) { - const local = {}; + const local = someFunc(); useEffect(() => { console.log(props.foo); console.log(props.bar); @@ -243,7 +243,7 @@ const tests = { console.log(props.bar); }, [props, props.foo]); - let color = {} + let color = someFunc(); useEffect(() => { console.log(props.foo.bar.baz); console.log(color); @@ -416,7 +416,7 @@ const tests = { { code: normalizeIndent` function MyComponent() { - const local = {}; + const local = someFunc(); function myEffect() { console.log(local); } @@ -731,7 +731,7 @@ const tests = { // direct assignments. code: normalizeIndent` function MyComponent(props) { - let obj = {}; + let obj = someFunc(); useEffect(() => { obj.foo = true; }, [obj]); @@ -1376,6 +1376,64 @@ const tests = { } `, }, + { + code: normalizeIndent` + function useFoo(foo){ + return useMemo(() => foo, [foo]); + } + `, + }, + { + code: normalizeIndent` + function useFoo(){ + const foo = "hi!"; + return useMemo(() => foo, [foo]); + } + `, + }, + { + code: normalizeIndent` + function useFoo(){ + let {foo} = {foo: 1}; + return useMemo(() => foo, [foo]); + } + `, + }, + { + code: normalizeIndent` + function useFoo(){ + let [foo] = [1]; + return useMemo(() => foo, [foo]); + } + `, + }, + { + code: normalizeIndent` + function useFoo() { + const foo = "fine"; + if (true) { + // Shadowed variable with constant construction in a nested scope is fine. + const foo = {}; + } + return useMemo(() => foo, [foo]); + } + `, + }, + { + code: normalizeIndent` + function MyComponent({foo}) { + return useMemo(() => foo, [foo]) + } + `, + }, + { + code: normalizeIndent` + function MyComponent() { + const foo = true ? "fine" : "also fine"; + return useMemo(() => foo, [foo]); + } + `, + }, ], invalid: [ { @@ -1494,7 +1552,7 @@ const tests = { { code: normalizeIndent` function MyComponent() { - const local = {}; + const local = someFunc(); useEffect(() => { console.log(local); }, []); @@ -1510,7 +1568,7 @@ const tests = { desc: 'Update the dependencies array to be: [local]', output: normalizeIndent` function MyComponent() { - const local = {}; + const local = someFunc(); useEffect(() => { console.log(local); }, [local]); @@ -1636,7 +1694,7 @@ const tests = { // Regression test code: normalizeIndent` function MyComponent() { - const local = {}; + const local = someFunc(); useEffect(() => { if (true) { console.log(local); @@ -1654,7 +1712,7 @@ const tests = { desc: 'Update the dependencies array to be: [local]', output: normalizeIndent` function MyComponent() { - const local = {}; + const local = someFunc(); useEffect(() => { if (true) { console.log(local); @@ -1742,9 +1800,9 @@ const tests = { { code: normalizeIndent` function MyComponent() { - const local1 = {}; + const local1 = someFunc(); { - const local2 = {}; + const local2 = someFunc(); useEffect(() => { console.log(local1); console.log(local2); @@ -1762,9 +1820,9 @@ const tests = { desc: 'Update the dependencies array to be: [local1, local2]', output: normalizeIndent` function MyComponent() { - const local1 = {}; + const local1 = someFunc(); { - const local2 = {}; + const local2 = someFunc(); useEffect(() => { console.log(local1); console.log(local2); @@ -1846,7 +1904,7 @@ const tests = { { code: normalizeIndent` function MyComponent() { - const local1 = {}; + const local1 = someFunc(); function MyNestedComponent() { const local2 = {}; useCallback(() => { @@ -1868,7 +1926,7 @@ const tests = { desc: 'Update the dependencies array to be: [local2]', output: normalizeIndent` function MyComponent() { - const local1 = {}; + const local1 = someFunc(); function MyNestedComponent() { const local2 = {}; useCallback(() => { @@ -2295,7 +2353,7 @@ const tests = { { code: normalizeIndent` function MyComponent() { - const local = {}; + const local = someFunc(); useEffect(() => { console.log(local); }, [local, ...dependencies]); @@ -5311,7 +5369,7 @@ const tests = { `The 'handleNext' function makes the dependencies of ` + `useEffect Hook (at line 11) change on every render. ` + `Move it inside the useEffect callback. Alternatively, ` + - `wrap the 'handleNext' definition into its own useCallback() Hook.`, + `wrap the definition of 'handleNext' in its own useCallback() Hook.`, // Not gonna fix a function definition // because it's not always safe due to hoisting. suggestions: undefined, @@ -5340,7 +5398,7 @@ const tests = { `The 'handleNext' function makes the dependencies of ` + `useEffect Hook (at line 11) change on every render. ` + `Move it inside the useEffect callback. Alternatively, ` + - `wrap the 'handleNext' definition into its own useCallback() Hook.`, + `wrap the definition of 'handleNext' in its own useCallback() Hook.`, // We don't fix moving (too invasive). But that's the suggested fix // when only effect uses this function. Otherwise, we'd useCallback. suggestions: undefined, @@ -5373,13 +5431,13 @@ const tests = { message: `The 'handleNext' function makes the dependencies of ` + `useEffect Hook (at line 11) change on every render. ` + - `To fix this, wrap the 'handleNext' definition into its own useCallback() Hook.`, + `To fix this, wrap the definition of 'handleNext' in its own useCallback() Hook.`, // We fix this one with useCallback since it's // the easy fix and you can't just move it into effect. suggestions: [ { desc: - "Wrap the 'handleNext' definition into its own useCallback() Hook.", + "Wrap the definition of 'handleNext' in its own useCallback() Hook.", output: normalizeIndent` function MyComponent(props) { let [, setState] = useState(); @@ -5428,21 +5486,21 @@ const tests = { message: "The 'handleNext1' function makes the dependencies of useEffect Hook " + '(at line 14) change on every render. Move it inside the useEffect callback. ' + - "Alternatively, wrap the 'handleNext1' definition into its own useCallback() Hook.", + "Alternatively, wrap the definition of 'handleNext1' in its own useCallback() Hook.", suggestions: undefined, }, { message: "The 'handleNext2' function makes the dependencies of useLayoutEffect Hook " + '(at line 17) change on every render. Move it inside the useLayoutEffect callback. ' + - "Alternatively, wrap the 'handleNext2' definition into its own useCallback() Hook.", + "Alternatively, wrap the definition of 'handleNext2' in its own useCallback() Hook.", suggestions: undefined, }, { message: "The 'handleNext3' function makes the dependencies of useMemo Hook " + '(at line 20) change on every render. Move it inside the useMemo callback. ' + - "Alternatively, wrap the 'handleNext3' definition into its own useCallback() Hook.", + "Alternatively, wrap the definition of 'handleNext3' in its own useCallback() Hook.", suggestions: undefined, }, ], @@ -5480,21 +5538,21 @@ const tests = { message: "The 'handleNext1' function makes the dependencies of useEffect Hook " + '(at line 15) change on every render. Move it inside the useEffect callback. ' + - "Alternatively, wrap the 'handleNext1' definition into its own useCallback() Hook.", + "Alternatively, wrap the definition of 'handleNext1' in its own useCallback() Hook.", suggestions: undefined, }, { message: "The 'handleNext2' function makes the dependencies of useLayoutEffect Hook " + '(at line 19) change on every render. Move it inside the useLayoutEffect callback. ' + - "Alternatively, wrap the 'handleNext2' definition into its own useCallback() Hook.", + "Alternatively, wrap the definition of 'handleNext2' in its own useCallback() Hook.", suggestions: undefined, }, { message: "The 'handleNext3' function makes the dependencies of useMemo Hook " + '(at line 23) change on every render. Move it inside the useMemo callback. ' + - "Alternatively, wrap the 'handleNext3' definition into its own useCallback() Hook.", + "Alternatively, wrap the definition of 'handleNext3' in its own useCallback() Hook.", suggestions: undefined, }, ], @@ -5541,20 +5599,20 @@ const tests = { message: "The 'handleNext1' function makes the dependencies of useEffect Hook " + '(at line 15) change on every render. To fix this, wrap the ' + - "'handleNext1' definition into its own useCallback() Hook.", + "definition of 'handleNext1' in its own useCallback() Hook.", suggestions: undefined, }, { message: "The 'handleNext2' function makes the dependencies of useLayoutEffect Hook " + '(at line 19) change on every render. To fix this, wrap the ' + - "'handleNext2' definition into its own useCallback() Hook.", + "definition of 'handleNext2' in its own useCallback() Hook.", // Suggestion wraps into useCallback where possible (variables only) // because they are only referenced outside the effect. suggestions: [ { desc: - "Wrap the 'handleNext2' definition into its own useCallback() Hook.", + "Wrap the definition of 'handleNext2' in its own useCallback() Hook.", output: normalizeIndent` function MyComponent(props) { function handleNext1() { @@ -5598,13 +5656,13 @@ const tests = { message: "The 'handleNext3' function makes the dependencies of useMemo Hook " + '(at line 23) change on every render. To fix this, wrap the ' + - "'handleNext3' definition into its own useCallback() Hook.", + "definition of 'handleNext3' in its own useCallback() Hook.", // Autofix wraps into useCallback where possible (variables only) // because they are only referenced outside the effect. suggestions: [ { desc: - "Wrap the 'handleNext3' definition into its own useCallback() Hook.", + "Wrap the definition of 'handleNext3' in its own useCallback() Hook.", output: normalizeIndent` function MyComponent(props) { function handleNext1() { @@ -5675,11 +5733,11 @@ const tests = { message: "The 'handleNext1' function makes the dependencies of useEffect Hook " + '(at line 12) change on every render. To fix this, wrap the ' + - "'handleNext1' definition into its own useCallback() Hook.", + "definition of 'handleNext1' in its own useCallback() Hook.", suggestions: [ { desc: - "Wrap the 'handleNext1' definition into its own useCallback() Hook.", + "Wrap the definition of 'handleNext1' in its own useCallback() Hook.", output: normalizeIndent` function MyComponent(props) { const handleNext1 = useCallback(() => { @@ -5705,11 +5763,11 @@ const tests = { message: "The 'handleNext1' function makes the dependencies of useEffect Hook " + '(at line 16) change on every render. To fix this, wrap the ' + - "'handleNext1' definition into its own useCallback() Hook.", + "definition of 'handleNext1' in its own useCallback() Hook.", suggestions: [ { desc: - "Wrap the 'handleNext1' definition into its own useCallback() Hook.", + "Wrap the definition of 'handleNext1' in its own useCallback() Hook.", output: normalizeIndent` function MyComponent(props) { const handleNext1 = useCallback(() => { @@ -5735,14 +5793,14 @@ const tests = { message: "The 'handleNext2' function makes the dependencies of useEffect Hook " + '(at line 12) change on every render. To fix this, wrap the ' + - "'handleNext2' definition into its own useCallback() Hook.", + "definition of 'handleNext2' in its own useCallback() Hook.", suggestions: undefined, }, { message: "The 'handleNext2' function makes the dependencies of useEffect Hook " + '(at line 16) change on every render. To fix this, wrap the ' + - "'handleNext2' definition into its own useCallback() Hook.", + "definition of 'handleNext2' in its own useCallback() Hook.", suggestions: undefined, }, ], @@ -5767,8 +5825,8 @@ const tests = { { message: "The 'handleNext' function makes the dependencies of useEffect Hook " + - '(at line 13) change on every render. To fix this, wrap the ' + - "'handleNext' definition into its own useCallback() Hook.", + '(at line 13) change on every render. To fix this, wrap the definition of ' + + "'handleNext' in its own useCallback() Hook.", // Normally we'd suggest moving handleNext inside an // effect. But it's used more than once. // TODO: our autofix here isn't quite sufficient because @@ -5776,7 +5834,7 @@ const tests = { suggestions: [ { desc: - "Wrap the 'handleNext' definition into its own useCallback() Hook.", + "Wrap the definition of 'handleNext' in its own useCallback() Hook.", output: normalizeIndent` function MyComponent(props) { let handleNext = useCallback(() => { @@ -5820,7 +5878,7 @@ const tests = { `The 'handleNext' function makes the dependencies of ` + `useEffect Hook (at line 14) change on every render. ` + `Move it inside the useEffect callback. Alternatively, wrap the ` + - `'handleNext' definition into its own useCallback() Hook.`, + `definition of 'handleNext' in its own useCallback() Hook.`, suggestions: undefined, }, ], @@ -6085,7 +6143,7 @@ const tests = { message: `The 'increment' function makes the dependencies of useEffect Hook ` + `(at line 14) change on every render. Move it inside the useEffect callback. ` + - `Alternatively, wrap the \'increment\' definition into its own ` + + `Alternatively, wrap the definition of \'increment\' in its own ` + `useCallback() Hook.`, suggestions: undefined, }, @@ -6575,7 +6633,7 @@ const tests = { ' }\n' + ' fetchData();\n' + `}, [someId]); // Or [] if effect doesn't need props or state\n\n` + - 'Learn more about data fetching with Hooks: https://fb.me/react-hooks-data-fetching', + 'Learn more about data fetching with Hooks: https://reactjs.org/link/hooks-data-fetching', suggestions: undefined, }, ], @@ -6599,7 +6657,7 @@ const tests = { ' }\n' + ' fetchData();\n' + `}, [someId]); // Or [] if effect doesn't need props or state\n\n` + - 'Learn more about data fetching with Hooks: https://fb.me/react-hooks-data-fetching', + 'Learn more about data fetching with Hooks: https://reactjs.org/link/hooks-data-fetching', suggestions: undefined, }, ], @@ -6967,6 +7025,499 @@ const tests = { }, ], }, + { + code: normalizeIndent` + function Component() { + const foo = {}; + useMemo(() => foo, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' object makes the dependencies of useMemo Hook (at line 4) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own " + + 'useMemo() Hook.', + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Component() { + const foo = []; + useMemo(() => foo, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' array makes the dependencies of useMemo Hook (at line 4) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own " + + 'useMemo() Hook.', + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Component() { + const foo = () => {}; + useMemo(() => foo, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' function makes the dependencies of useMemo Hook (at line 4) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the definition of 'foo' in its own " + + 'useCallback() Hook.', + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Component() { + const foo = function bar(){}; + useMemo(() => foo, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' function makes the dependencies of useMemo Hook (at line 4) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the definition of 'foo' in its own " + + 'useCallback() Hook.', + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Component() { + const foo = class {}; + useMemo(() => foo, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' class makes the dependencies of useMemo Hook (at line 4) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own " + + 'useMemo() Hook.', + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Component() { + const foo = true ? {} : "fine"; + useMemo(() => foo, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' conditional could make the dependencies of useMemo Hook (at line 4) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own " + + 'useMemo() Hook.', + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Component() { + const foo = bar || {}; + useMemo(() => foo, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' logical expression could make the dependencies of useMemo Hook (at line 4) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own " + + 'useMemo() Hook.', + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Component() { + const foo = bar ?? {}; + useMemo(() => foo, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' logical expression could make the dependencies of useMemo Hook (at line 4) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own " + + 'useMemo() Hook.', + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Component() { + const foo = bar && {}; + useMemo(() => foo, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' logical expression could make the dependencies of useMemo Hook (at line 4) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own " + + 'useMemo() Hook.', + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Component() { + const foo = bar ? baz ? {} : null : null; + useMemo(() => foo, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' conditional could make the dependencies of useMemo Hook (at line 4) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own " + + 'useMemo() Hook.', + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Component() { + let foo = {}; + useMemo(() => foo, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' object makes the dependencies of useMemo Hook (at line 4) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own " + + 'useMemo() Hook.', + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Component() { + var foo = {}; + useMemo(() => foo, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' object makes the dependencies of useMemo Hook (at line 4) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own " + + 'useMemo() Hook.', + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Component() { + const foo = {}; + useCallback(() => { + console.log(foo); + }, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' object makes the dependencies of useCallback Hook (at line 6) change on every render. " + + "Move it inside the useCallback callback. Alternatively, wrap the initialization of 'foo' in its own " + + 'useMemo() Hook.', + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Component() { + const foo = {}; + useEffect(() => { + console.log(foo); + }, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' object makes the dependencies of useEffect Hook (at line 6) change on every render. " + + "Move it inside the useEffect callback. Alternatively, wrap the initialization of 'foo' in its own " + + 'useMemo() Hook.', + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Component() { + const foo = {}; + useLayoutEffect(() => { + console.log(foo); + }, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' object makes the dependencies of useLayoutEffect Hook (at line 6) change on every render. " + + "Move it inside the useLayoutEffect callback. Alternatively, wrap the initialization of 'foo' in its own " + + 'useMemo() Hook.', + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Component() { + const foo = {}; + useImperativeHandle( + ref, + () => { + console.log(foo); + }, + [foo] + ); + } + `, + errors: [ + { + message: + "The 'foo' object makes the dependencies of useImperativeHandle Hook (at line 9) change on every render. " + + "Move it inside the useImperativeHandle callback. Alternatively, wrap the initialization of 'foo' in its own " + + 'useMemo() Hook.', + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Foo(section) { + const foo = section.section_components?.edges ?? []; + useEffect(() => { + console.log(foo); + }, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' logical expression could make the dependencies of useEffect Hook (at line 6) change on every render. " + + "Move it inside the useEffect callback. Alternatively, wrap the initialization of 'foo' in its own " + + 'useMemo() Hook.', + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Foo(section) { + const foo = {}; + console.log(foo); + useMemo(() => { + console.log(foo); + }, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' object makes the dependencies of useMemo Hook (at line 7) change on every render. " + + "To fix this, wrap the initialization of 'foo' in its own useMemo() Hook.", + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Foo() { + const foo = <>Hi!; + useMemo(() => { + console.log(foo); + }, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' JSX fragment makes the dependencies of useMemo Hook (at line 6) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own useMemo() Hook.", + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Foo() { + const foo =
Hi!
; + useMemo(() => { + console.log(foo); + }, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' JSX element makes the dependencies of useMemo Hook (at line 6) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own useMemo() Hook.", + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Foo() { + const foo = bar = {}; + useMemo(() => { + console.log(foo); + }, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' assignment expression makes the dependencies of useMemo Hook (at line 6) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own useMemo() Hook.", + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Foo() { + const foo = new String('foo'); // Note 'foo' will be boxed, and thus an object and thus compared by reference. + useMemo(() => { + console.log(foo); + }, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' object construction makes the dependencies of useMemo Hook (at line 6) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own useMemo() Hook.", + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Foo() { + const foo = new Map([]); + useMemo(() => { + console.log(foo); + }, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' object construction makes the dependencies of useMemo Hook (at line 6) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own useMemo() Hook.", + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Foo() { + const foo = /reg/; + useMemo(() => { + console.log(foo); + }, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' regular expression makes the dependencies of useMemo Hook (at line 6) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own useMemo() Hook.", + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Foo() { + const foo = ({}: any); + useMemo(() => { + console.log(foo); + }, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' object makes the dependencies of useMemo Hook (at line 6) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own useMemo() Hook.", + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Foo() { + class Bar {}; + useMemo(() => { + console.log(new Bar()); + }, [Bar]); + } + `, + errors: [ + { + message: + "The 'Bar' class makes the dependencies of useMemo Hook (at line 6) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'Bar' in its own useMemo() Hook.", + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Foo() { + const foo = {}; + useLayoutEffect(() => { + console.log(foo); + }, [foo]); + useEffect(() => { + console.log(foo); + }, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' object makes the dependencies of useLayoutEffect Hook (at line 6) change on every render. " + + "To fix this, wrap the initialization of 'foo' in its own useMemo() Hook.", + suggestions: undefined, + }, + { + message: + "The 'foo' object makes the dependencies of useEffect Hook (at line 9) change on every render. " + + "To fix this, wrap the initialization of 'foo' in its own useMemo() Hook.", + suggestions: undefined, + }, + ], + }, ], }; @@ -7345,6 +7896,24 @@ const testsTypescript = { }, ], }, + { + code: normalizeIndent` + function Foo() { + const foo = {} as any; + useMemo(() => { + console.log(foo); + }, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' object makes the dependencies of useMemo Hook (at line 6) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own useMemo() Hook.", + suggestions: undefined, + }, + ], + }, ], }; diff --git a/packages/eslint-plugin-react-hooks/package.json b/packages/eslint-plugin-react-hooks/package.json index 9ebde813a8c3..884f62d7a904 100644 --- a/packages/eslint-plugin-react-hooks/package.json +++ b/packages/eslint-plugin-react-hooks/package.json @@ -1,7 +1,7 @@ { "name": "eslint-plugin-react-hooks", "description": "ESLint rules for React Hooks", - "version": "4.0.8", + "version": "4.1.0", "repository": { "type": "git", "url": "https://github.com/facebook/react.git", diff --git a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js index 9e344e8662d6..ed48afa7869c 100644 --- a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js +++ b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js @@ -258,7 +258,7 @@ export default { ' }\n' + ' fetchData();\n' + `}, [someId]); // Or [] if effect doesn't need props or state\n\n` + - 'Learn more about data fetching with Hooks: https://fb.me/react-hooks-data-fetching', + 'Learn more about data fetching with Hooks: https://reactjs.org/link/hooks-data-fetching', }); } @@ -844,59 +844,80 @@ export default { unnecessaryDependencies.size; if (problemCount === 0) { - // If nothing else to report, check if some callbacks - // are bare and would invalidate on every render. - const bareFunctions = scanForDeclaredBareFunctions({ + // If nothing else to report, check if some dependencies would + // invalidate on every render. + const constructions = scanForConstructions({ declaredDependencies, declaredDependenciesNode, componentScope, scope, }); - bareFunctions.forEach(({fn, suggestUseCallback}) => { - let message = - `The '${fn.name.name}' function makes the dependencies of ` + - `${reactiveHookName} Hook (at line ${declaredDependenciesNode.loc.start.line}) ` + - `change on every render.`; - if (suggestUseCallback) { - message += - ` To fix this, ` + - `wrap the '${fn.name.name}' definition into its own useCallback() Hook.`; - } else { - message += - ` Move it inside the ${reactiveHookName} callback. ` + - `Alternatively, wrap the '${fn.name.name}' definition into its own useCallback() Hook.`; - } + constructions.forEach( + ({construction, isUsedOutsideOfHook, depType}) => { + const wrapperHook = + depType === 'function' ? 'useCallback' : 'useMemo'; - let suggest; - // Only handle the simple case: arrow functions. - // Wrapping function declarations can mess up hoisting. - if (suggestUseCallback && fn.type === 'Variable') { - suggest = [ - { - desc: `Wrap the '${fn.name.name}' definition into its own useCallback() Hook.`, - fix(fixer) { - return [ - // TODO: also add an import? - fixer.insertTextBefore(fn.node.init, 'useCallback('), - // TODO: ideally we'd gather deps here but it would require - // restructuring the rule code. This will cause a new lint - // error to appear immediately for useCallback. Note we're - // not adding [] because would that changes semantics. - fixer.insertTextAfter(fn.node.init, ')'), - ]; + const constructionType = + depType === 'function' ? 'definition' : 'initialization'; + + const defaultAdvice = `wrap the ${constructionType} of '${construction.name.name}' in its own ${wrapperHook}() Hook.`; + + const advice = isUsedOutsideOfHook + ? `To fix this, ${defaultAdvice}` + : `Move it inside the ${reactiveHookName} callback. Alternatively, ${defaultAdvice}`; + + const causation = + depType === 'conditional' || depType === 'logical expression' + ? 'could make' + : 'makes'; + + const message = + `The '${construction.name.name}' ${depType} ${causation} the dependencies of ` + + `${reactiveHookName} Hook (at line ${declaredDependenciesNode.loc.start.line}) ` + + `change on every render. ${advice}`; + + let suggest; + // Only handle the simple case of variable assignments. + // Wrapping function declarations can mess up hoisting. + if ( + isUsedOutsideOfHook && + construction.type === 'Variable' && + // Objects may be mutated ater construction, which would make this + // fix unsafe. Functions _probably_ won't be mutated, so we'll + // allow this fix for them. + depType === 'function' + ) { + suggest = [ + { + desc: `Wrap the ${constructionType} of '${construction.name.name}' in its own ${wrapperHook}() Hook.`, + fix(fixer) { + const [before, after] = + wrapperHook === 'useMemo' + ? [`useMemo(() => { return `, '; })'] + : ['useCallback(', ')']; + return [ + // TODO: also add an import? + fixer.insertTextBefore(construction.node.init, before), + // TODO: ideally we'd gather deps here but it would require + // restructuring the rule code. This will cause a new lint + // error to appear immediately for useCallback. Note we're + // not adding [] because would that changes semantics. + fixer.insertTextAfter(construction.node.init, after), + ]; + }, }, - }, - ]; - } - // TODO: What if the function needs to change on every render anyway? - // Should we suggest removing effect deps as an appropriate fix too? - reportProblem({ - // TODO: Why not report this at the dependency site? - node: fn.node, - message, - suggest, - }); - }); + ]; + } + // TODO: What if the function needs to change on every render anyway? + // Should we suggest removing effect deps as an appropriate fix too? + reportProblem({ + // TODO: Why not report this at the dependency site? + node: construction.node, + message, + suggest, + }); + }, + ); return; } @@ -1381,50 +1402,116 @@ function collectRecommendations({ }; } -// Finds functions declared as dependencies +// If the node will result in constructing a referentially unique value, return +// its human readable type name, else return null. +function getConstructionExpressionType(node) { + switch (node.type) { + case 'ObjectExpression': + return 'object'; + case 'ArrayExpression': + return 'array'; + case 'ArrowFunctionExpression': + case 'FunctionExpression': + return 'function'; + case 'ClassExpression': + return 'class'; + case 'ConditionalExpression': + if ( + getConstructionExpressionType(node.consequent) != null || + getConstructionExpressionType(node.alternate) != null + ) { + return 'conditional'; + } + return null; + case 'LogicalExpression': + if ( + getConstructionExpressionType(node.left) != null || + getConstructionExpressionType(node.right) != null + ) { + return 'logical expression'; + } + return null; + case 'JSXFragment': + return 'JSX fragment'; + case 'JSXElement': + return 'JSX element'; + case 'AssignmentExpression': + if (getConstructionExpressionType(node.right) != null) { + return 'assignment expression'; + } + return null; + case 'NewExpression': + return 'object construction'; + case 'Literal': + if (node.value instanceof RegExp) { + return 'regular expression'; + } + return null; + case 'TypeCastExpression': + return getConstructionExpressionType(node.expression); + case 'TSAsExpression': + return getConstructionExpressionType(node.expression); + } + return null; +} + +// Finds variables declared as dependencies // that would invalidate on every render. -function scanForDeclaredBareFunctions({ +function scanForConstructions({ declaredDependencies, declaredDependenciesNode, componentScope, scope, }) { - const bareFunctions = declaredDependencies + const constructions = declaredDependencies .map(({key}) => { - const fnRef = componentScope.set.get(key); - if (fnRef == null) { + const ref = componentScope.variables.find(v => v.name === key); + if (ref == null) { return null; } - const fnNode = fnRef.defs[0]; - if (fnNode == null) { + + const node = ref.defs[0]; + if (node == null) { return null; } // const handleChange = function () {} // const handleChange = () => {} + // const foo = {} + // const foo = [] + // etc. if ( - fnNode.type === 'Variable' && - fnNode.node.type === 'VariableDeclarator' && - fnNode.node.init != null && - (fnNode.node.init.type === 'ArrowFunctionExpression' || - fnNode.node.init.type === 'FunctionExpression') + node.type === 'Variable' && + node.node.type === 'VariableDeclarator' && + node.node.id.type === 'Identifier' && // Ensure this is not destructed assignment + node.node.init != null ) { - return fnRef; + const constantExpressionType = getConstructionExpressionType( + node.node.init, + ); + if (constantExpressionType != null) { + return [ref, constantExpressionType]; + } } // function handleChange() {} if ( - fnNode.type === 'FunctionName' && - fnNode.node.type === 'FunctionDeclaration' + node.type === 'FunctionName' && + node.node.type === 'FunctionDeclaration' ) { - return fnRef; + return [ref, 'function']; + } + + // class Foo {} + if (node.type === 'ClassName' && node.node.type === 'ClassDeclaration') { + return [ref, 'class']; } return null; }) .filter(Boolean); - function isUsedOutsideOfHook(fnRef) { + function isUsedOutsideOfHook(ref) { let foundWriteExpr = false; - for (let i = 0; i < fnRef.references.length; i++) { - const reference = fnRef.references[i]; + for (let i = 0; i < ref.references.length; i++) { + const reference = ref.references[i]; if (reference.writeExpr) { if (foundWriteExpr) { // Two writes to the same function. @@ -1450,9 +1537,10 @@ function scanForDeclaredBareFunctions({ return false; } - return bareFunctions.map(fnRef => ({ - fn: fnRef.defs[0], - suggestUseCallback: isUsedOutsideOfHook(fnRef), + return constructions.map(([ref, depType]) => ({ + construction: ref.defs[0], + depType, + isUsedOutsideOfHook: isUsedOutsideOfHook(ref), })); } diff --git a/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.js b/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.js index 49786ddfd8ac..602fb613d417 100644 --- a/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.js +++ b/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.js @@ -284,7 +284,7 @@ describe('ReactHooksInspection', () => { '1. You might have mismatching versions of React and the renderer (such as React DOM)\n' + '2. You might be breaking the Rules of Hooks\n' + '3. You might have more than one copy of React in the same app\n' + - 'See https://fb.me/react-invalid-hook-call for tips about how to debug and fix this problem.', + 'See https://reactjs.org/link/invalid-hook-call for tips about how to debug and fix this problem.', ); expect(getterCalls).toBe(1); diff --git a/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js b/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js index 7bffd1d80965..01461dfc1648 100644 --- a/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js +++ b/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js @@ -787,7 +787,7 @@ describe('ReactHooksInspectionIntegration', () => { '1. You might have mismatching versions of React and the renderer (such as React DOM)\n' + '2. You might be breaking the Rules of Hooks\n' + '3. You might have more than one copy of React in the same app\n' + - 'See https://fb.me/react-invalid-hook-call for tips about how to debug and fix this problem.', + 'See https://reactjs.org/link/invalid-hook-call for tips about how to debug and fix this problem.', ); expect(getterCalls).toBe(1); diff --git a/packages/react-devtools-extensions/popups/shared.js b/packages/react-devtools-extensions/popups/shared.js index ddb0456e5bbf..53b2bc1d16e2 100644 --- a/packages/react-devtools-extensions/popups/shared.js +++ b/packages/react-devtools-extensions/popups/shared.js @@ -11,6 +11,7 @@ document.addEventListener('DOMContentLoaded', function() { const location = ln.href; ln.onclick = function() { chrome.tabs.create({active: true, url: location}); + return false; }; })(); } diff --git a/packages/react-devtools-extensions/src/backend.js b/packages/react-devtools-extensions/src/backend.js index 357c286c2e28..aa8578a10864 100644 --- a/packages/react-devtools-extensions/src/backend.js +++ b/packages/react-devtools-extensions/src/backend.js @@ -72,7 +72,7 @@ function setup(hook) { initBackend(hook, agent, window); // Let the frontend know that the backend has attached listeners and is ready for messages. - // This covers the case of of syncing saved values after reloading/navigating while DevTools remain open. + // This covers the case of syncing saved values after reloading/navigating while DevTools remain open. bridge.send('extensionBackendInitialized'); // Setup React Native style editor if a renderer like react-native-web has injected it. diff --git a/packages/react-devtools-extensions/src/main.js b/packages/react-devtools-extensions/src/main.js index a405255ff59e..00e3735c72c1 100644 --- a/packages/react-devtools-extensions/src/main.js +++ b/packages/react-devtools-extensions/src/main.js @@ -296,7 +296,7 @@ function createPanelIfReactLoaded() { let needsToSyncElementSelection = false; chrome.devtools.panels.create( - isChrome ? '⚛ Components' : 'Components', + isChrome ? '⚛️ Components' : 'Components', '', 'panel.html', extensionPanel => { @@ -326,7 +326,7 @@ function createPanelIfReactLoaded() { ); chrome.devtools.panels.create( - isChrome ? '⚛ Profiler' : 'Profiler', + isChrome ? '⚛️ Profiler' : 'Profiler', '', 'panel.html', extensionPanel => { diff --git a/packages/react-devtools-shared/src/__tests__/__snapshots__/inspectedElementContext-test.js.snap b/packages/react-devtools-shared/src/__tests__/__snapshots__/inspectedElementContext-test.js.snap index 2ef217a451e6..1989aee6391a 100644 --- a/packages/react-devtools-shared/src/__tests__/__snapshots__/inspectedElementContext-test.js.snap +++ b/packages/react-devtools-shared/src/__tests__/__snapshots__/inspectedElementContext-test.js.snap @@ -541,6 +541,7 @@ exports[`InspectedElementContext should support complex data types: 1: Inspected "object_of_objects": { "inner": {} }, + "proxy": {}, "react_element": {}, "regexp": {}, "set": { diff --git a/packages/react-devtools-shared/src/__tests__/__snapshots__/ownersListContext-test.js.snap b/packages/react-devtools-shared/src/__tests__/__snapshots__/ownersListContext-test.js.snap index 0699eb5b18dc..25e56daf6f44 100644 --- a/packages/react-devtools-shared/src/__tests__/__snapshots__/ownersListContext-test.js.snap +++ b/packages/react-devtools-shared/src/__tests__/__snapshots__/ownersListContext-test.js.snap @@ -78,6 +78,33 @@ Array [ ] `; +exports[`OwnersListContext should include all owners for a component wrapped in react memo: owners for "InnerComponent" 1`] = ` +Array [ + Object { + "displayName": "Grandparent", + "hocDisplayNames": null, + "id": 2, + "type": 5, + }, + Object { + "displayName": "InnerComponent", + "hocDisplayNames": Array [ + "Memo", + ], + "id": 3, + "type": 8, + }, + Object { + "displayName": "InnerComponent", + "hocDisplayNames": Array [ + "ForwardRef", + ], + "id": 4, + "type": 6, + }, +] +`; + exports[`OwnersListContext should include the current element even if there are no other owners: mount 1`] = ` [root] diff --git a/packages/react-devtools-shared/src/__tests__/inspectedElementContext-test.js b/packages/react-devtools-shared/src/__tests__/inspectedElementContext-test.js index 4e292ecdbe5a..29e8e379e4e4 100644 --- a/packages/react-devtools-shared/src/__tests__/inspectedElementContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/inspectedElementContext-test.js @@ -549,6 +549,14 @@ describe('InspectedElementContext', () => { } const instance = new Class(); + const proxyInstance = new Proxy(() => {}, { + get: function(_, name) { + return function() { + return null; + }; + }, + }); + const container = document.createElement('div'); await utils.actAsync(() => ReactDOM.render( @@ -567,6 +575,7 @@ describe('InspectedElementContext', () => { map={mapShallow} map_of_maps={mapOfMaps} object_of_objects={objectOfObjects} + proxy={proxyInstance} react_element={} regexp={/abc/giu} set={setShallow} @@ -619,6 +628,7 @@ describe('InspectedElementContext', () => { map, map_of_maps, object_of_objects, + proxy, react_element, regexp, set, @@ -722,6 +732,12 @@ describe('InspectedElementContext', () => { ); expect(object_of_objects.inner[meta.preview_short]).toBe('{…}'); + expect(proxy[meta.inspectable]).toBe(false); + expect(proxy[meta.name]).toBe('function'); + expect(proxy[meta.type]).toBe('function'); + expect(proxy[meta.preview_long]).toBe('ƒ () {}'); + expect(proxy[meta.preview_short]).toBe('ƒ () {}'); + expect(react_element[meta.inspectable]).toBe(false); expect(react_element[meta.name]).toBe('span'); expect(react_element[meta.type]).toBe('react_element'); diff --git a/packages/react-devtools-shared/src/__tests__/ownersListContext-test.js b/packages/react-devtools-shared/src/__tests__/ownersListContext-test.js index b39da7063206..4e60bd6a5502 100644 --- a/packages/react-devtools-shared/src/__tests__/ownersListContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/ownersListContext-test.js @@ -206,4 +206,43 @@ describe('OwnersListContext', () => { done(); }); + + it('should include all owners for a component wrapped in react memo', async done => { + const InnerComponent = (props, ref) =>
; + const ForwardRef = React.forwardRef(InnerComponent); + const Memo = React.memo(ForwardRef); + const Grandparent = () => { + const ref = React.createRef(); + return ; + }; + + utils.act(() => + ReactDOM.render(, document.createElement('div')), + ); + + let didFinish = false; + function Suspender({owner}) { + const read = React.useContext(OwnersListContext); + const owners = read(owner.id); + didFinish = true; + expect(owners.length).toBe(3); + expect(owners).toMatchSnapshot( + `owners for "${(owner && owner.displayName) || ''}"`, + ); + return null; + } + + const wrapped = ((store.getElementAtIndex(2): any): Element); + await utils.actAsync(() => + TestRenderer.create( + + + + + , + ), + ); + expect(didFinish).toBe(true); + done(); + }); }); diff --git a/packages/react-devtools-shared/src/devtools/views/Components/KeyValue.css b/packages/react-devtools-shared/src/devtools/views/Components/KeyValue.css index 2a40f54a2cc8..0824b5a2c4af 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/KeyValue.css +++ b/packages/react-devtools-shared/src/devtools/views/Components/KeyValue.css @@ -21,7 +21,7 @@ .Value { color: var(--color-attribute-value); - white-space: nowrap; + white-space: pre; overflow: hidden; text-overflow: ellipsis; cursor: default; diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/NoInteractions.js b/packages/react-devtools-shared/src/devtools/views/Profiler/NoInteractions.js index 932c906cc7c0..c2f1f65480d1 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/NoInteractions.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/NoInteractions.js @@ -24,7 +24,7 @@ export default function NoInteractions({

Learn more about the interaction tracing API here. diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/Profiler.js b/packages/react-devtools-shared/src/devtools/views/Profiler/Profiler.js index 2756ba0c9f83..2b4e117d66d9 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/Profiler.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/Profiler.js @@ -176,10 +176,10 @@ const ProfilingNotSupported = () => ( Learn more at{' '} - fb.me/react-profiling + reactjs.org/link/profiling .

diff --git a/packages/react-devtools-shared/src/hook.js b/packages/react-devtools-shared/src/hook.js index 4b018e61421b..f9f65df7fb9c 100644 --- a/packages/react-devtools-shared/src/hook.js +++ b/packages/react-devtools-shared/src/hook.js @@ -145,7 +145,7 @@ export function installHook(target: any): DevToolsHook | null { 'React is running in production mode, but dead code ' + 'elimination has not been applied. Read how to correctly ' + 'configure React for production: ' + - 'https://fb.me/react-perf-use-the-production-build', + 'https://reactjs.org/link/perf-use-production-build', ); }); } diff --git a/packages/react-devtools-shared/src/hydration.js b/packages/react-devtools-shared/src/hydration.js index f46048f75e64..23cb0a68fd9b 100644 --- a/packages/react-devtools-shared/src/hydration.js +++ b/packages/react-devtools-shared/src/hydration.js @@ -151,7 +151,10 @@ export function dehydrate( inspectable: false, preview_short: formatDataForPreview(data, false), preview_long: formatDataForPreview(data, true), - name: data.name || 'function', + name: + typeof data.name === 'function' || !data.name + ? 'function' + : data.name, type, }; diff --git a/packages/react-devtools-shared/src/utils.js b/packages/react-devtools-shared/src/utils.js index 3472e2c6dba9..419aaac0c3dd 100644 --- a/packages/react-devtools-shared/src/utils.js +++ b/packages/react-devtools-shared/src/utils.js @@ -539,7 +539,9 @@ export function formatDataForPreview( case 'html_element': return `<${truncateForDisplay(data.tagName.toLowerCase())} />`; case 'function': - return truncateForDisplay(`ƒ ${data.name}() {}`); + return truncateForDisplay( + `ƒ ${typeof data.name === 'function' ? '' : data.name}() {}`, + ); case 'string': return `"${data}"`; case 'bigint': diff --git a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js index 939b7b77203e..1a02d22d5dbf 100644 --- a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js +++ b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js @@ -801,7 +801,7 @@ describe('ReactComponentLifeCycle', () => { ' UNSAFE_componentWillReceiveProps\n' + ' componentWillUpdate\n\n' + 'The above lifecycles should be removed. Learn more about this warning here:\n' + - 'https://fb.me/react-unsafe-component-lifecycles', + 'https://reactjs.org/link/unsafe-component-lifecycles', ); }).toWarnDev( [ @@ -827,7 +827,7 @@ describe('ReactComponentLifeCycle', () => { 'WillMount uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + ' UNSAFE_componentWillMount\n\n' + 'The above lifecycles should be removed. Learn more about this warning here:\n' + - 'https://fb.me/react-unsafe-component-lifecycles', + 'https://reactjs.org/link/unsafe-component-lifecycles', ); class WillMountAndUpdate extends React.Component { @@ -851,7 +851,7 @@ describe('ReactComponentLifeCycle', () => { ' componentWillMount\n' + ' UNSAFE_componentWillUpdate\n\n' + 'The above lifecycles should be removed. Learn more about this warning here:\n' + - 'https://fb.me/react-unsafe-component-lifecycles', + 'https://reactjs.org/link/unsafe-component-lifecycles', ); }).toWarnDev(['componentWillMount has been renamed'], { withoutStack: true, @@ -874,7 +874,7 @@ describe('ReactComponentLifeCycle', () => { 'WillReceiveProps uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + ' componentWillReceiveProps\n\n' + 'The above lifecycles should be removed. Learn more about this warning here:\n' + - 'https://fb.me/react-unsafe-component-lifecycles', + 'https://reactjs.org/link/unsafe-component-lifecycles', ); }).toWarnDev(['componentWillReceiveProps has been renamed'], { withoutStack: true, @@ -906,7 +906,7 @@ describe('ReactComponentLifeCycle', () => { ' UNSAFE_componentWillReceiveProps\n' + ' componentWillUpdate\n\n' + 'The above lifecycles should be removed. Learn more about this warning here:\n' + - 'https://fb.me/react-unsafe-component-lifecycles', + 'https://reactjs.org/link/unsafe-component-lifecycles', ); }).toWarnDev( [ @@ -931,7 +931,7 @@ describe('ReactComponentLifeCycle', () => { 'WillMount uses getSnapshotBeforeUpdate() but also contains the following legacy lifecycles:\n' + ' UNSAFE_componentWillMount\n\n' + 'The above lifecycles should be removed. Learn more about this warning here:\n' + - 'https://fb.me/react-unsafe-component-lifecycles', + 'https://reactjs.org/link/unsafe-component-lifecycles', ); class WillMountAndUpdate extends React.Component { @@ -954,7 +954,7 @@ describe('ReactComponentLifeCycle', () => { ' componentWillMount\n' + ' UNSAFE_componentWillUpdate\n\n' + 'The above lifecycles should be removed. Learn more about this warning here:\n' + - 'https://fb.me/react-unsafe-component-lifecycles', + 'https://reactjs.org/link/unsafe-component-lifecycles', ); }).toWarnDev(['componentWillMount has been renamed'], { withoutStack: true, @@ -976,7 +976,7 @@ describe('ReactComponentLifeCycle', () => { 'WillReceiveProps uses getSnapshotBeforeUpdate() but also contains the following legacy lifecycles:\n' + ' componentWillReceiveProps\n\n' + 'The above lifecycles should be removed. Learn more about this warning here:\n' + - 'https://fb.me/react-unsafe-component-lifecycles', + 'https://reactjs.org/link/unsafe-component-lifecycles', ); }).toWarnDev(['componentWillReceiveProps has been renamed'], { withoutStack: true, @@ -1383,20 +1383,20 @@ describe('ReactComponentLifeCycle', () => { expect(() => ReactDOM.render(, container)).toWarnDev( [ /* eslint-disable max-len */ - `Warning: componentWillMount has been renamed, and is not recommended for use. See https://fb.me/react-unsafe-component-lifecycles for details. + `Warning: componentWillMount has been renamed, and is not recommended for use. See https://reactjs.org/link/unsafe-component-lifecycles for details. * Move code with side effects to componentDidMount, and set initial state in the constructor. * Rename componentWillMount to UNSAFE_componentWillMount to suppress this warning in non-strict mode. In React 18.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, you can run \`npx react-codemod rename-unsafe-lifecycles\` in your project source folder. Please update the following components: MyComponent`, - `Warning: componentWillReceiveProps has been renamed, and is not recommended for use. See https://fb.me/react-unsafe-component-lifecycles for details. + `Warning: componentWillReceiveProps has been renamed, and is not recommended for use. See https://reactjs.org/link/unsafe-component-lifecycles for details. * Move data fetching code or side effects to componentDidUpdate. -* If you're updating state whenever props change, refactor your code to use memoization techniques or move it to static getDerivedStateFromProps. Learn more at: https://fb.me/react-derived-state +* If you're updating state whenever props change, refactor your code to use memoization techniques or move it to static getDerivedStateFromProps. Learn more at: https://reactjs.org/link/derived-state * Rename componentWillReceiveProps to UNSAFE_componentWillReceiveProps to suppress this warning in non-strict mode. In React 18.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, you can run \`npx react-codemod rename-unsafe-lifecycles\` in your project source folder. Please update the following components: MyComponent`, - `Warning: componentWillUpdate has been renamed, and is not recommended for use. See https://fb.me/react-unsafe-component-lifecycles for details. + `Warning: componentWillUpdate has been renamed, and is not recommended for use. See https://reactjs.org/link/unsafe-component-lifecycles for details. * Move data fetching code or side effects to componentDidUpdate. * Rename componentWillUpdate to UNSAFE_componentWillUpdate to suppress this warning in non-strict mode. In React 18.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, you can run \`npx react-codemod rename-unsafe-lifecycles\` in your project source folder. diff --git a/packages/react-dom/src/__tests__/ReactDOMAttribute-test.js b/packages/react-dom/src/__tests__/ReactDOMAttribute-test.js index efd01b41af73..aa91857b4800 100644 --- a/packages/react-dom/src/__tests__/ReactDOMAttribute-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMAttribute-test.js @@ -93,7 +93,7 @@ describe('ReactDOM unknown attribute', () => { expect(() => testUnknownAttributeRemoval(Symbol('foo'))).toErrorDev( 'Warning: Invalid value for prop `unknown` on
tag. Either remove it ' + 'from the element, or pass a string or number value to keep it ' + - 'in the DOM. For details, see https://fb.me/react-attribute-behavior\n' + + 'in the DOM. For details, see https://reactjs.org/link/attribute-behavior \n' + ' in div (at **)', ); }); @@ -105,7 +105,7 @@ describe('ReactDOM unknown attribute', () => { 'Warning: Invalid value for prop `unknown` on
tag. Either remove ' + 'it from the element, or pass a string or number value to ' + 'keep it in the DOM. For details, see ' + - 'https://fb.me/react-attribute-behavior\n' + + 'https://reactjs.org/link/attribute-behavior \n' + ' in div (at **)', ); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js index 3c8ce165ebb9..d4e23c926733 100644 --- a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js @@ -150,7 +150,7 @@ describe('ReactDOMComponent', () => { ).toErrorDev( 'Warning: Invalid value for prop `foo` on
tag. Either remove it ' + 'from the element, or pass a string or number value to keep ' + - 'it in the DOM. For details, see https://fb.me/react-attribute-behavior' + + 'it in the DOM. For details, see https://reactjs.org/link/attribute-behavior ' + '\n in div (at **)', ); }); @@ -162,7 +162,7 @@ describe('ReactDOMComponent', () => { ).toErrorDev( 'Warning: Invalid values for props `foo`, `baz` on
tag. Either remove ' + 'them from the element, or pass a string or number value to keep ' + - 'them in the DOM. For details, see https://fb.me/react-attribute-behavior' + + 'them in the DOM. For details, see https://reactjs.org/link/attribute-behavior ' + '\n in div (at **)', ); }); @@ -1384,7 +1384,7 @@ describe('ReactDOMComponent', () => { mountComponent({dangerouslySetInnerHTML: 'Hi Jim!'}); }).toThrowError( '`props.dangerouslySetInnerHTML` must be in the form `{__html: ...}`. ' + - 'Please visit https://fb.me/react-invariant-dangerously-set-inner-html for more information.', + 'Please visit https://reactjs.org/link/dangerously-set-inner-html for more information.', ); }); @@ -1393,7 +1393,7 @@ describe('ReactDOMComponent', () => { mountComponent({dangerouslySetInnerHTML: {foo: 'bar'}}); }).toThrowError( '`props.dangerouslySetInnerHTML` must be in the form `{__html: ...}`. ' + - 'Please visit https://fb.me/react-invariant-dangerously-set-inner-html for more information.', + 'Please visit https://reactjs.org/link/dangerously-set-inner-html for more information.', ); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMComponentTree-test.js b/packages/react-dom/src/__tests__/ReactDOMComponentTree-test.js index 218c2a81c066..647c6d527992 100644 --- a/packages/react-dom/src/__tests__/ReactDOMComponentTree-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMComponentTree-test.js @@ -183,7 +183,7 @@ describe('ReactDOMComponentTree', () => { 'a defined value, which should not happen. ' + 'Decide between using a controlled or uncontrolled input ' + 'element for the lifetime of the component. More info: ' + - 'https://fb.me/react-controlled-components', + 'https://reactjs.org/link/controlled-components', ); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js index cb28d2f25226..5cb207409da7 100644 --- a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js @@ -712,12 +712,23 @@ describe('ReactDOMEventListener', () => { bubbles: false, }), ); - expect(log).toEqual([ - ['capture', 'grand'], - ['capture', 'parent'], - ['capture', 'child'], - ['bubble', 'child'], - ]); + if (gate(flags => flags.disableOnScrollBubbling)) { + expect(log).toEqual([ + ['capture', 'grand'], + ['capture', 'parent'], + ['capture', 'child'], + ['bubble', 'child'], + ]); + } else { + expect(log).toEqual([ + ['capture', 'grand'], + ['capture', 'parent'], + ['capture', 'child'], + ['bubble', 'child'], + ['bubble', 'parent'], + ['bubble', 'grand'], + ]); + } } finally { document.body.removeChild(container); } diff --git a/packages/react-dom/src/__tests__/ReactDOMEventPropagation-test.js b/packages/react-dom/src/__tests__/ReactDOMEventPropagation-test.js index 1c737df509fb..0ef1a8b950d3 100644 --- a/packages/react-dom/src/__tests__/ReactDOMEventPropagation-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMEventPropagation-test.js @@ -50,6 +50,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onAnimationEnd', + reactEventType: 'animationend', nativeEvent: 'animationend', dispatch(node) { node.dispatchEvent( @@ -66,6 +67,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onAnimationIteration', + reactEventType: 'animationiteration', nativeEvent: 'animationiteration', dispatch(node) { node.dispatchEvent( @@ -82,6 +84,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onAnimationStart', + reactEventType: 'animationstart', nativeEvent: 'animationstart', dispatch(node) { node.dispatchEvent( @@ -98,6 +101,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onAuxClick', + reactEventType: 'auxclick', nativeEvent: 'auxclick', dispatch(node) { node.dispatchEvent( @@ -114,6 +118,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'input', reactEvent: 'onBlur', + reactEventType: 'blur', nativeEvent: 'focusout', dispatch(node) { const e = new Event('focusout', { @@ -133,6 +138,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onClick', + reactEventType: 'click', nativeEvent: 'click', dispatch(node) { node.click(); @@ -144,6 +150,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onContextMenu', + reactEventType: 'contextmenu', nativeEvent: 'contextmenu', dispatch(node) { node.dispatchEvent( @@ -160,6 +167,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onCopy', + reactEventType: 'copy', nativeEvent: 'copy', dispatch(node) { node.dispatchEvent( @@ -176,6 +184,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onCut', + reactEventType: 'cut', nativeEvent: 'cut', dispatch(node) { node.dispatchEvent( @@ -192,6 +201,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onDoubleClick', + reactEventType: 'dblclick', nativeEvent: 'dblclick', dispatch(node) { node.dispatchEvent( @@ -208,6 +218,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onDrag', + reactEventType: 'drag', nativeEvent: 'drag', dispatch(node) { node.dispatchEvent( @@ -224,6 +235,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onDragEnd', + reactEventType: 'dragend', nativeEvent: 'dragend', dispatch(node) { node.dispatchEvent( @@ -240,6 +252,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onDragEnter', + reactEventType: 'dragenter', nativeEvent: 'dragenter', dispatch(node) { node.dispatchEvent( @@ -256,6 +269,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onDragExit', + reactEventType: 'dragexit', nativeEvent: 'dragexit', dispatch(node) { node.dispatchEvent( @@ -272,6 +286,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onDragLeave', + reactEventType: 'dragleave', nativeEvent: 'dragleave', dispatch(node) { node.dispatchEvent( @@ -288,6 +303,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onDragOver', + reactEventType: 'dragover', nativeEvent: 'dragover', dispatch(node) { node.dispatchEvent( @@ -304,6 +320,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onDragStart', + reactEventType: 'dragstart', nativeEvent: 'dragstart', dispatch(node) { node.dispatchEvent( @@ -320,6 +337,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onDrop', + reactEventType: 'drop', nativeEvent: 'drop', dispatch(node) { node.dispatchEvent( @@ -336,6 +354,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'input', reactEvent: 'onFocus', + reactEventType: 'focus', nativeEvent: 'focusin', dispatch(node) { const e = new Event('focusin', { @@ -351,6 +370,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onGotPointerCapture', + reactEventType: 'gotpointercapture', nativeEvent: 'gotpointercapture', dispatch(node) { node.dispatchEvent( @@ -367,6 +387,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'input', reactEvent: 'onKeyDown', + reactEventType: 'keydown', nativeEvent: 'keydown', dispatch(node) { node.dispatchEvent( @@ -383,6 +404,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'input', reactEvent: 'onKeyPress', + reactEventType: 'keypress', nativeEvent: 'keypress', dispatch(node) { node.dispatchEvent( @@ -400,6 +422,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'input', reactEvent: 'onKeyUp', + reactEventType: 'keyup', nativeEvent: 'keyup', dispatch(node) { node.dispatchEvent( @@ -416,6 +439,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onLostPointerCapture', + reactEventType: 'lostpointercapture', nativeEvent: 'lostpointercapture', dispatch(node) { node.dispatchEvent( @@ -432,6 +456,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onMouseDown', + reactEventType: 'mousedown', nativeEvent: 'mousedown', dispatch(node) { node.dispatchEvent( @@ -448,6 +473,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onMouseOut', + reactEventType: 'mouseout', nativeEvent: 'mouseout', dispatch(node) { node.dispatchEvent( @@ -464,6 +490,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onMouseOver', + reactEventType: 'mouseover', nativeEvent: 'mouseover', dispatch(node) { node.dispatchEvent( @@ -480,6 +507,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onMouseUp', + reactEventType: 'mouseup', nativeEvent: 'mouseup', dispatch(node) { node.dispatchEvent( @@ -496,6 +524,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onPaste', + reactEventType: 'paste', nativeEvent: 'paste', dispatch(node) { node.dispatchEvent( @@ -512,6 +541,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onPointerCancel', + reactEventType: 'pointercancel', nativeEvent: 'pointercancel', dispatch(node) { node.dispatchEvent( @@ -528,6 +558,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onPointerDown', + reactEventType: 'pointerdown', nativeEvent: 'pointerdown', dispatch(node) { node.dispatchEvent( @@ -544,6 +575,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onPointerMove', + reactEventType: 'pointermove', nativeEvent: 'pointermove', dispatch(node) { node.dispatchEvent( @@ -560,6 +592,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onPointerOut', + reactEventType: 'pointerout', nativeEvent: 'pointerout', dispatch(node) { node.dispatchEvent( @@ -576,6 +609,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onPointerOver', + reactEventType: 'pointerover', nativeEvent: 'pointerover', dispatch(node) { node.dispatchEvent( @@ -592,6 +626,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onPointerUp', + reactEventType: 'pointerup', nativeEvent: 'pointerup', dispatch(node) { node.dispatchEvent( @@ -608,6 +643,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'form', reactEvent: 'onReset', + reactEventType: 'reset', nativeEvent: 'reset', dispatch(node) { const e = new Event('reset', { @@ -623,6 +659,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'form', reactEvent: 'onSubmit', + reactEventType: 'submit', nativeEvent: 'submit', dispatch(node) { const e = new Event('submit', { @@ -638,6 +675,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onTouchCancel', + reactEventType: 'touchcancel', nativeEvent: 'touchcancel', dispatch(node) { node.dispatchEvent( @@ -654,6 +692,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onTouchEnd', + reactEventType: 'touchend', nativeEvent: 'touchend', dispatch(node) { node.dispatchEvent( @@ -670,6 +709,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onTouchMove', + reactEventType: 'touchmove', nativeEvent: 'touchmove', dispatch(node) { node.dispatchEvent( @@ -686,6 +726,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onTouchStart', + reactEventType: 'touchstart', nativeEvent: 'touchstart', dispatch(node) { node.dispatchEvent( @@ -702,6 +743,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onTransitionEnd', + reactEventType: 'transitionend', nativeEvent: 'transitionend', dispatch(node) { node.dispatchEvent( @@ -718,6 +760,7 @@ describe('ReactDOMEventListener', () => { testNativeBubblingEvent({ type: 'div', reactEvent: 'onWheel', + reactEventType: 'wheel', nativeEvent: 'wheel', dispatch(node) { node.dispatchEvent( @@ -736,6 +779,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'video', reactEvent: 'onAbort', + reactEventType: 'abort', nativeEvent: 'abort', dispatch(node) { const e = new Event('abort', { @@ -751,6 +795,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'dialog', reactEvent: 'onCancel', + reactEventType: 'cancel', nativeEvent: 'cancel', dispatch(node) { const e = new Event('cancel', { @@ -766,6 +811,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'video', reactEvent: 'onCanPlay', + reactEventType: 'canplay', nativeEvent: 'canplay', dispatch(node) { const e = new Event('canplay', { @@ -781,6 +827,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'video', reactEvent: 'onCanPlayThrough', + reactEventType: 'canplaythrough', nativeEvent: 'canplaythrough', dispatch(node) { const e = new Event('canplaythrough', { @@ -796,6 +843,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'dialog', reactEvent: 'onClose', + reactEventType: 'close', nativeEvent: 'close', dispatch(node) { const e = new Event('close', { @@ -811,6 +859,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'video', reactEvent: 'onDurationChange', + reactEventType: 'durationchange', nativeEvent: 'durationchange', dispatch(node) { const e = new Event('durationchange', { @@ -826,6 +875,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'video', reactEvent: 'onEmptied', + reactEventType: 'emptied', nativeEvent: 'emptied', dispatch(node) { const e = new Event('emptied', { @@ -841,6 +891,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'video', reactEvent: 'onEncrypted', + reactEventType: 'encrypted', nativeEvent: 'encrypted', dispatch(node) { const e = new Event('encrypted', { @@ -856,6 +907,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'video', reactEvent: 'onEnded', + reactEventType: 'ended', nativeEvent: 'ended', dispatch(node) { const e = new Event('ended', { @@ -871,6 +923,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'img', reactEvent: 'onError', + reactEventType: 'error', nativeEvent: 'error', dispatch(node) { const e = new Event('error', { @@ -886,6 +939,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'input', reactEvent: 'onInvalid', + reactEventType: 'invalid', nativeEvent: 'invalid', dispatch(node) { const e = new Event('invalid', { @@ -901,6 +955,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'img', reactEvent: 'onLoad', + reactEventType: 'load', nativeEvent: 'load', dispatch(node) { const e = new Event('load', { @@ -916,6 +971,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'video', reactEvent: 'onLoadedData', + reactEventType: 'loadeddata', nativeEvent: 'loadeddata', dispatch(node) { const e = new Event('loadeddata', { @@ -931,6 +987,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'video', reactEvent: 'onLoadedMetadata', + reactEventType: 'loadedmetadata', nativeEvent: 'loadedmetadata', dispatch(node) { const e = new Event('loadedmetadata', { @@ -946,6 +1003,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'video', reactEvent: 'onLoadStart', + reactEventType: 'loadstart', nativeEvent: 'loadstart', dispatch(node) { const e = new Event('loadstart', { @@ -961,6 +1019,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'video', reactEvent: 'onPause', + reactEventType: 'pause', nativeEvent: 'pause', dispatch(node) { const e = new Event('pause', { @@ -976,6 +1035,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'video', reactEvent: 'onPlay', + reactEventType: 'play', nativeEvent: 'play', dispatch(node) { const e = new Event('play', { @@ -991,6 +1051,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'video', reactEvent: 'onPlaying', + reactEventType: 'playing', nativeEvent: 'playing', dispatch(node) { const e = new Event('playing', { @@ -1006,6 +1067,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'video', reactEvent: 'onProgress', + reactEventType: 'progress', nativeEvent: 'progress', dispatch(node) { const e = new Event('progress', { @@ -1021,6 +1083,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'video', reactEvent: 'onRateChange', + reactEventType: 'ratechange', nativeEvent: 'ratechange', dispatch(node) { const e = new Event('ratechange', { @@ -1036,6 +1099,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'video', reactEvent: 'onSeeked', + reactEventType: 'seeked', nativeEvent: 'seeked', dispatch(node) { const e = new Event('seeked', { @@ -1051,6 +1115,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'video', reactEvent: 'onSeeking', + reactEventType: 'seeking', nativeEvent: 'seeking', dispatch(node) { const e = new Event('seeking', { @@ -1066,6 +1131,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'video', reactEvent: 'onStalled', + reactEventType: 'stalled', nativeEvent: 'stalled', dispatch(node) { const e = new Event('stalled', { @@ -1081,6 +1147,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'video', reactEvent: 'onSuspend', + reactEventType: 'suspend', nativeEvent: 'suspend', dispatch(node) { const e = new Event('suspend', { @@ -1096,6 +1163,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'video', reactEvent: 'onTimeUpdate', + reactEventType: 'timeupdate', nativeEvent: 'timeupdate', dispatch(node) { const e = new Event('timeupdate', { @@ -1111,6 +1179,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'details', reactEvent: 'onToggle', + reactEventType: 'toggle', nativeEvent: 'toggle', dispatch(node) { const e = new Event('toggle', { @@ -1126,6 +1195,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'video', reactEvent: 'onVolumeChange', + reactEventType: 'volumechange', nativeEvent: 'volumechange', dispatch(node) { const e = new Event('volumechange', { @@ -1141,6 +1211,7 @@ describe('ReactDOMEventListener', () => { testEmulatedBubblingEvent({ type: 'video', reactEvent: 'onWaiting', + reactEventType: 'waiting', nativeEvent: 'waiting', dispatch(node) { const e = new Event('waiting', { @@ -1154,10 +1225,17 @@ describe('ReactDOMEventListener', () => { }); describe('non-bubbling events that do not bubble in React', () => { + // This test will fail outside of the no-bubbling flag + // because its bubbling emulation is currently broken. + // In particular, if the target itself doesn't have + // a handler, it will not emulate bubbling correctly. + // Instead of fixing this, we'll just turn this flag on. + // @gate disableOnScrollBubbling it('onScroll', () => { testNonBubblingEvent({ type: 'div', reactEvent: 'onScroll', + reactEventType: 'scroll', nativeEvent: 'scroll', dispatch(node) { const e = new Event('scroll', { @@ -1368,6 +1446,7 @@ describe('ReactDOMEventListener', () => { log.push('- outer parent'); }, onBeforeInputCapture: e => { + expect(e.type).toBe('beforeinput'); log.push('- outer parent capture'); }, }} @@ -1430,6 +1509,7 @@ describe('ReactDOMEventListener', () => { log.push('- outer parent'); }, onChangeCapture: e => { + expect(e.type).toBe('change'); log.push('- outer parent capture'); }, }} @@ -1488,6 +1568,7 @@ describe('ReactDOMEventListener', () => { log.push('- outer parent'); }, onCompositionStartCapture: e => { + expect(e.type).toBe('compositionstart'); log.push('- outer parent capture'); }, }} @@ -1549,6 +1630,7 @@ describe('ReactDOMEventListener', () => { log.push('- outer parent'); }, onCompositionEndCapture: e => { + expect(e.type).toBe('compositionend'); log.push('- outer parent capture'); }, }} @@ -1610,6 +1692,7 @@ describe('ReactDOMEventListener', () => { log.push('- outer parent'); }, onCompositionUpdateCapture: e => { + expect(e.type).toBe('compositionupdate'); log.push('- outer parent capture'); }, }} @@ -1671,6 +1754,7 @@ describe('ReactDOMEventListener', () => { log.push('- outer parent'); }, onSelectCapture: e => { + expect(e.type).toBe('select'); log.push('- outer parent capture'); }, }} @@ -1769,6 +1853,7 @@ describe('ReactDOMEventListener', () => { log.push('- outer parent'); }, [eventConfig.reactEvent + 'Capture']: e => { + expect(e.type).toBe(eventConfig.reactEventType); log.push('- outer parent capture'); }, }} @@ -1825,6 +1910,7 @@ describe('ReactDOMEventListener', () => { log.push('- outer parent'); }, [eventConfig.reactEvent + 'Capture']: e => { + expect(e.type).toBe(eventConfig.reactEventType); log.push('- outer parent capture'); }, }} @@ -1883,6 +1969,7 @@ describe('ReactDOMEventListener', () => { log.push('- outer parent'); }, [eventConfig.reactEvent + 'Capture']: e => { + expect(e.type).toBe(eventConfig.reactEventType); log.push('- outer parent capture'); }, }} @@ -1935,6 +2022,7 @@ describe('ReactDOMEventListener', () => { log.push('- outer parent'); }, [eventConfig.reactEvent + 'Capture']: e => { + expect(e.type).toBe(eventConfig.reactEventType); log.push('- outer parent capture'); }, }} @@ -1986,6 +2074,7 @@ describe('ReactDOMEventListener', () => { log.push('- outer parent'); }, [eventConfig.reactEvent + 'Capture']: e => { + expect(e.type).toBe(eventConfig.reactEventType); log.push('- outer parent capture'); }, }} @@ -2039,6 +2128,7 @@ describe('ReactDOMEventListener', () => { log.push('- outer parent'); }, [eventConfig.reactEvent + 'Capture']: e => { + expect(e.type).toBe(eventConfig.reactEventType); log.push('- outer parent capture'); }, }} @@ -2103,6 +2193,7 @@ describe('ReactDOMEventListener', () => { log.push('- outer parent'); }, [eventConfig.reactEvent + 'Capture']: e => { + expect(e.type).toBe(eventConfig.reactEventType); log.push('- outer parent capture'); }, }} @@ -2165,6 +2256,7 @@ describe('ReactDOMEventListener', () => { log.push('- outer parent'); }, [eventConfig.reactEvent + 'Capture']: e => { + expect(e.type).toBe(eventConfig.reactEventType); log.push('- outer parent capture'); }, }} @@ -2228,6 +2320,7 @@ describe('ReactDOMEventListener', () => { log.push('- outer parent'); }, [eventConfig.reactEvent + 'Capture']: e => { + expect(e.type).toBe(eventConfig.reactEventType); log.push('- outer parent capture'); }, }} @@ -2282,6 +2375,7 @@ describe('ReactDOMEventListener', () => { log.push('- outer parent'); }, [eventConfig.reactEvent + 'Capture']: e => { + expect(e.type).toBe(eventConfig.reactEventType); log.push('- outer parent capture'); }, }} @@ -2350,6 +2444,7 @@ describe('ReactDOMEventListener', () => { log.push('- outer parent'); }, [eventConfig.reactEvent + 'Capture']: e => { + expect(e.type).toBe(eventConfig.reactEventType); log.push('- outer parent capture'); }, }} @@ -2417,6 +2512,7 @@ describe('ReactDOMEventListener', () => { log.push('- outer parent'); }, [eventConfig.reactEvent + 'Capture']: e => { + expect(e.type).toBe(eventConfig.reactEventType); log.push('- outer parent capture'); }, }} @@ -2482,6 +2578,7 @@ describe('ReactDOMEventListener', () => { log.push('- outer parent'); }, [eventConfig.reactEvent + 'Capture']: e => { + expect(e.type).toBe(eventConfig.reactEventType); log.push('- outer parent capture'); }, }} @@ -2547,6 +2644,7 @@ describe('ReactDOMEventListener', () => { log.push('- outer parent'); }, [eventConfig.reactEvent + 'Capture']: e => { + expect(e.type).toBe(eventConfig.reactEventType); log.push('- outer parent capture'); }, }} @@ -2615,6 +2713,7 @@ describe('ReactDOMEventListener', () => { log.push('- outer parent'); }, [eventConfig.reactEvent + 'Capture']: e => { + expect(e.type).toBe(eventConfig.reactEventType); log.push('- outer parent capture'); }, }} diff --git a/packages/react-dom/src/__tests__/ReactDOMFiber-test.js b/packages/react-dom/src/__tests__/ReactDOMFiber-test.js index abf076597c63..b05b4c1f9614 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFiber-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFiber-test.js @@ -989,6 +989,57 @@ describe('ReactDOMFiber', () => { } }); + // Regression test for https://github.com/facebook/react/issues/19562 + it('does not fire mouseEnter twice when relatedTarget is the root node', () => { + let ops = []; + let target = null; + + function simulateMouseMove(from, to) { + if (from) { + from.dispatchEvent( + new MouseEvent('mouseout', { + bubbles: true, + cancelable: true, + relatedTarget: to, + }), + ); + } + if (to) { + to.dispatchEvent( + new MouseEvent('mouseover', { + bubbles: true, + cancelable: true, + relatedTarget: from, + }), + ); + } + } + + ReactDOM.render( +
(target = n)} + onMouseEnter={() => ops.push('enter')} + onMouseLeave={() => ops.push('leave')} + />, + container, + ); + + simulateMouseMove(null, container); + expect(ops).toEqual([]); + + ops = []; + simulateMouseMove(container, target); + expect(ops).toEqual(['enter']); + + ops = []; + simulateMouseMove(target, container); + expect(ops).toEqual(['leave']); + + ops = []; + simulateMouseMove(container, null); + expect(ops).toEqual([]); + }); + it('should throw on bad createPortal argument', () => { expect(() => { ReactDOM.createPortal(
portal
, null); diff --git a/packages/react-dom/src/__tests__/ReactDOMInput-test.js b/packages/react-dom/src/__tests__/ReactDOMInput-test.js index f78e11830e4d..018dd14697f0 100644 --- a/packages/react-dom/src/__tests__/ReactDOMInput-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMInput-test.js @@ -1228,7 +1228,7 @@ describe('ReactDOMInput', () => { '(specify either the checked prop, or the defaultChecked prop, but not ' + 'both). Decide between using a controlled or uncontrolled input ' + 'element and remove one of these props. More info: ' + - 'https://fb.me/react-controlled-components', + 'https://reactjs.org/link/controlled-components', ); ReactDOM.unmountComponentAtNode(container); @@ -1255,7 +1255,7 @@ describe('ReactDOMInput', () => { '(specify either the value prop, or the defaultValue prop, but not ' + 'both). Decide between using a controlled or uncontrolled input ' + 'element and remove one of these props. More info: ' + - 'https://fb.me/react-controlled-components', + 'https://reactjs.org/link/controlled-components', ); ReactDOM.unmountComponentAtNode(container); @@ -1275,7 +1275,7 @@ describe('ReactDOMInput', () => { 'This is likely caused by the value changing from a defined to ' + 'undefined, which should not happen. ' + 'Decide between using a controlled or uncontrolled input ' + - 'element for the lifetime of the component. More info: https://fb.me/react-controlled-components\n' + + 'element for the lifetime of the component. More info: https://reactjs.org/link/controlled-components\n' + ' in input (at **)', ); }); @@ -1294,7 +1294,7 @@ describe('ReactDOMInput', () => { 'This is likely caused by the value changing from a defined to ' + 'undefined, which should not happen. ' + 'Decide between using a controlled or uncontrolled input ' + - 'element for the lifetime of the component. More info: https://fb.me/react-controlled-components\n' + + 'element for the lifetime of the component. More info: https://reactjs.org/link/controlled-components\n' + ' in input (at **)', ]); }); @@ -1314,7 +1314,7 @@ describe('ReactDOMInput', () => { 'This is likely caused by the value changing from a defined to ' + 'undefined, which should not happen. ' + 'Decide between using a controlled or uncontrolled input ' + - 'element for the lifetime of the component. More info: https://fb.me/react-controlled-components\n' + + 'element for the lifetime of the component. More info: https://reactjs.org/link/controlled-components\n' + ' in input (at **)', ); }); @@ -1329,7 +1329,7 @@ describe('ReactDOMInput', () => { 'This is likely caused by the value changing from undefined to ' + 'a defined value, which should not happen. ' + 'Decide between using a controlled or uncontrolled input ' + - 'element for the lifetime of the component. More info: https://fb.me/react-controlled-components\n' + + 'element for the lifetime of the component. More info: https://reactjs.org/link/controlled-components\n' + ' in input (at **)', ); }); @@ -1347,7 +1347,7 @@ describe('ReactDOMInput', () => { 'This is likely caused by the value changing from undefined to ' + 'a defined value, which should not happen. ' + 'Decide between using a controlled or uncontrolled input ' + - 'element for the lifetime of the component. More info: https://fb.me/react-controlled-components\n' + + 'element for the lifetime of the component. More info: https://reactjs.org/link/controlled-components\n' + ' in input (at **)', ); }); @@ -1364,7 +1364,7 @@ describe('ReactDOMInput', () => { 'This is likely caused by the value changing from a defined to ' + 'undefined, which should not happen. ' + 'Decide between using a controlled or uncontrolled input ' + - 'element for the lifetime of the component. More info: https://fb.me/react-controlled-components\n' + + 'element for the lifetime of the component. More info: https://reactjs.org/link/controlled-components\n' + ' in input (at **)', ); }); @@ -1381,7 +1381,7 @@ describe('ReactDOMInput', () => { 'This is likely caused by the value changing from a defined to ' + 'undefined, which should not happen. ' + 'Decide between using a controlled or uncontrolled input ' + - 'element for the lifetime of the component. More info: https://fb.me/react-controlled-components\n' + + 'element for the lifetime of the component. More info: https://reactjs.org/link/controlled-components\n' + ' in input (at **)', ); }); @@ -1401,7 +1401,7 @@ describe('ReactDOMInput', () => { 'This is likely caused by the value changing from a defined to ' + 'undefined, which should not happen. ' + 'Decide between using a controlled or uncontrolled input ' + - 'element for the lifetime of the component. More info: https://fb.me/react-controlled-components\n' + + 'element for the lifetime of the component. More info: https://reactjs.org/link/controlled-components\n' + ' in input (at **)', ); }); @@ -1416,7 +1416,7 @@ describe('ReactDOMInput', () => { 'This is likely caused by the value changing from undefined to ' + 'a defined value, which should not happen. ' + 'Decide between using a controlled or uncontrolled input ' + - 'element for the lifetime of the component. More info: https://fb.me/react-controlled-components\n' + + 'element for the lifetime of the component. More info: https://reactjs.org/link/controlled-components\n' + ' in input (at **)', ); }); @@ -1431,7 +1431,7 @@ describe('ReactDOMInput', () => { 'This is likely caused by the value changing from undefined to ' + 'a defined value, which should not happen. ' + 'Decide between using a controlled or uncontrolled input ' + - 'element for the lifetime of the component. More info: https://fb.me/react-controlled-components\n' + + 'element for the lifetime of the component. More info: https://reactjs.org/link/controlled-components\n' + ' in input (at **)', ); }); @@ -1444,7 +1444,7 @@ describe('ReactDOMInput', () => { 'This is likely caused by the value changing from a defined to ' + 'undefined, which should not happen. ' + 'Decide between using a controlled or uncontrolled input ' + - 'element for the lifetime of the component. More info: https://fb.me/react-controlled-components\n' + + 'element for the lifetime of the component. More info: https://reactjs.org/link/controlled-components\n' + ' in input (at **)', ); }); @@ -1459,7 +1459,7 @@ describe('ReactDOMInput', () => { 'This is likely caused by the value changing from a defined to ' + 'undefined, which should not happen. ' + 'Decide between using a controlled or uncontrolled input ' + - 'element for the lifetime of the component. More info: https://fb.me/react-controlled-components\n' + + 'element for the lifetime of the component. More info: https://reactjs.org/link/controlled-components\n' + ' in input (at **)', ); }); @@ -1474,7 +1474,7 @@ describe('ReactDOMInput', () => { 'This is likely caused by the value changing from a defined to ' + 'undefined, which should not happen. ' + 'Decide between using a controlled or uncontrolled input ' + - 'element for the lifetime of the component. More info: https://fb.me/react-controlled-components\n' + + 'element for the lifetime of the component. More info: https://reactjs.org/link/controlled-components\n' + ' in input (at **)', ); }); @@ -1489,7 +1489,7 @@ describe('ReactDOMInput', () => { 'This is likely caused by the value changing from undefined to ' + 'a defined value, which should not happen. ' + 'Decide between using a controlled or uncontrolled input ' + - 'element for the lifetime of the component. More info: https://fb.me/react-controlled-components\n' + + 'element for the lifetime of the component. More info: https://reactjs.org/link/controlled-components\n' + ' in input (at **)', ); }); @@ -1504,7 +1504,7 @@ describe('ReactDOMInput', () => { 'This is likely caused by the value changing from undefined to ' + 'a defined value, which should not happen. ' + 'Decide between using a controlled or uncontrolled input ' + - 'element for the lifetime of the component. More info: https://fb.me/react-controlled-components\n' + + 'element for the lifetime of the component. More info: https://reactjs.org/link/controlled-components\n' + ' in input (at **)', ); }); @@ -1557,7 +1557,7 @@ describe('ReactDOMInput', () => { 'This is likely caused by the value changing from a defined to ' + 'undefined, which should not happen. ' + 'Decide between using a controlled or uncontrolled input ' + - 'element for the lifetime of the component. More info: https://fb.me/react-controlled-components\n' + + 'element for the lifetime of the component. More info: https://reactjs.org/link/controlled-components\n' + ' in input (at **)', ); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMInvalidARIAHook-test.js b/packages/react-dom/src/__tests__/ReactDOMInvalidARIAHook-test.js index 7263aac40529..f3d48aca792f 100644 --- a/packages/react-dom/src/__tests__/ReactDOMInvalidARIAHook-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMInvalidARIAHook-test.js @@ -31,7 +31,7 @@ describe('ReactDOMInvalidARIAHook', () => { it('should warn for one invalid aria-* prop', () => { expect(() => mountComponent({'aria-badprop': 'maybe'})).toErrorDev( 'Warning: Invalid aria prop `aria-badprop` on
tag. ' + - 'For details, see https://fb.me/invalid-aria-prop', + 'For details, see https://reactjs.org/link/invalid-aria-props', ); }); it('should warn for many invalid aria-* props', () => { @@ -42,7 +42,7 @@ describe('ReactDOMInvalidARIAHook', () => { }), ).toErrorDev( 'Warning: Invalid aria props `aria-badprop`, `aria-malprop` on
' + - 'tag. For details, see https://fb.me/invalid-aria-prop', + 'tag. For details, see https://reactjs.org/link/invalid-aria-props', ); }); it('should warn for an improperly cased aria-* prop', () => { diff --git a/packages/react-dom/src/__tests__/ReactDOMSelect-test.js b/packages/react-dom/src/__tests__/ReactDOMSelect-test.js index 61315f0a95fd..b9d76492c73b 100644 --- a/packages/react-dom/src/__tests__/ReactDOMSelect-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMSelect-test.js @@ -693,7 +693,7 @@ describe('ReactDOMSelect', () => { '(specify either the value prop, or the defaultValue prop, but not ' + 'both). Decide between using a controlled or uncontrolled select ' + 'element and remove one of these props. More info: ' + - 'https://fb.me/react-controlled-components', + 'https://reactjs.org/link/controlled-components', ); ReactTestUtils.renderIntoDocument( diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js index 813c0fe1e7ad..0497a696716c 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js @@ -156,7 +156,7 @@ describe('ReactDOMServerHooks', () => { '1. You might have mismatching versions of React and the renderer (such as React DOM)\n' + '2. You might be breaking the Rules of Hooks\n' + '3. You might have more than one copy of React in the same app\n' + - 'See https://fb.me/react-invalid-hook-call for tips about how to debug and fix this problem.', + 'See https://reactjs.org/link/invalid-hook-call for tips about how to debug and fix this problem.', ); itRenders('multiple times when an updater is called', async render => { @@ -674,7 +674,7 @@ describe('ReactDOMServerHooks', () => { '1. You might have mismatching versions of React and the renderer (such as React DOM)\n' + '2. You might be breaking the Rules of Hooks\n' + '3. You might have more than one copy of React in the same app\n' + - 'See https://fb.me/react-invalid-hook-call for tips about how to debug and fix this problem.', + 'See https://reactjs.org/link/invalid-hook-call for tips about how to debug and fix this problem.', ); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationUntrustedURL-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationUntrustedURL-test.internal.js index 0148249bd10f..9fb48b3bce24 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationUntrustedURL-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationUntrustedURL-test.internal.js @@ -250,7 +250,7 @@ describe('ReactDOMServerIntegration - Untrusted URLs - disableJavaScriptURLs', ( // the first times it is called and then becomes dangerous. toStringCalls++; if (toStringCalls <= expectedToStringCalls) { - return 'https://fb.me/'; + return 'https://reactjs.org/'; } return 'javascript:notfine'; }, @@ -258,7 +258,7 @@ describe('ReactDOMServerIntegration - Untrusted URLs - disableJavaScriptURLs', ( const e = await render(); expect(toStringCalls).toBe(expectedToStringCalls); - expect(e.href).toBe('https://fb.me/'); + expect(e.href).toBe('https://reactjs.org/'); }); it('rejects a javascript protocol href if it is added during an update twice', () => { diff --git a/packages/react-dom/src/__tests__/ReactDOMServerLifecycles-test.js b/packages/react-dom/src/__tests__/ReactDOMServerLifecycles-test.js index 377ced34db98..5f1875f5f431 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerLifecycles-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerLifecycles-test.js @@ -293,7 +293,7 @@ describe('ReactDOMServerLifecycles', () => { } expect(() => ReactDOMServer.renderToString()).toWarnDev( - 'componentWillMount has been renamed, and is not recommended for use. See https://fb.me/react-unsafe-component-lifecycles for details.\n\n' + + 'componentWillMount has been renamed, and is not recommended for use. See https://reactjs.org/link/unsafe-component-lifecycles for details.\n\n' + '* Move code from componentWillMount to componentDidMount (preferred in most cases) or the constructor.\n\n' + 'Please update the following components: MyComponent', ); diff --git a/packages/react-dom/src/__tests__/ReactDOMTextarea-test.js b/packages/react-dom/src/__tests__/ReactDOMTextarea-test.js index 92d1142f5b18..4d56f53d9e40 100644 --- a/packages/react-dom/src/__tests__/ReactDOMTextarea-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMTextarea-test.js @@ -525,7 +525,7 @@ describe('ReactDOMTextarea', () => { '(specify either the value prop, or the defaultValue prop, but not ' + 'both). Decide between using a controlled or uncontrolled textarea ' + 'and remove one of these props. More info: ' + - 'https://fb.me/react-controlled-components', + 'https://reactjs.org/link/controlled-components', ); // No additional warnings are expected diff --git a/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.internal.js b/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.internal.js index 5ccbd7e481e4..f65c6e85e834 100644 --- a/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.internal.js @@ -69,7 +69,7 @@ describe('ReactDeprecationWarnings', () => { 'Support for string refs will be removed in a future major release. ' + 'We recommend using useRef() or createRef() instead. ' + 'Learn more about using refs safely here: ' + - 'https://fb.me/react-strict-mode-string-ref' + + 'https://reactjs.org/link/strict-mode-string-ref' + '\n in Component (at **)', ); }); @@ -110,7 +110,7 @@ describe('ReactDeprecationWarnings', () => { 'This case cannot be automatically converted to an arrow function. ' + 'We ask you to manually fix this case by using useRef() or createRef() instead. ' + 'Learn more about using refs safely here: ' + - 'https://fb.me/react-strict-mode-string-ref', + 'https://reactjs.org/link/strict-mode-string-ref', ]); }); @@ -141,7 +141,7 @@ describe('ReactDeprecationWarnings', () => { 'This case cannot be automatically converted to an arrow function. ' + 'We ask you to manually fix this case by using useRef() or createRef() instead. ' + 'Learn more about using refs safely here: ' + - 'https://fb.me/react-strict-mode-string-ref', + 'https://reactjs.org/link/strict-mode-string-ref', ); }); } diff --git a/packages/react-dom/src/__tests__/ReactEmptyComponent-test.js b/packages/react-dom/src/__tests__/ReactEmptyComponent-test.js index 10e95f7cf7ba..0c0b1d1a9273 100644 --- a/packages/react-dom/src/__tests__/ReactEmptyComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactEmptyComponent-test.js @@ -316,4 +316,24 @@ describe('ReactEmptyComponent', () => { const noscript2 = container.firstChild; expect(noscript2).toBe(null); }); + + it('should warn about React.forwardRef that returns undefined', () => { + const Empty = () => {}; + const EmptyForwardRef = React.forwardRef(Empty); + + expect(() => { + ReactTestUtils.renderIntoDocument(); + }).toThrowError( + 'ForwardRef(Empty)(...): Nothing was returned from render.', + ); + }); + + it('should warn about React.memo that returns undefined', () => { + const Empty = () => {}; + const EmptyMemo = React.memo(Empty); + + expect(() => { + ReactTestUtils.renderIntoDocument(); + }).toThrowError('Empty(...): Nothing was returned from render.'); + }); }); diff --git a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js index 06f51b0f3974..204c67ba5284 100644 --- a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js @@ -2473,4 +2473,175 @@ describe('ReactErrorBoundaries', () => { 'Caught an error: gotta catch em all.', ); }); + + // @gate skipUnmountedBoundaries + it('catches errors thrown in componentWillUnmount', () => { + class LocalErrorBoundary extends React.Component { + state = {error: null}; + static getDerivedStateFromError(error) { + Scheduler.unstable_yieldValue( + `ErrorBoundary static getDerivedStateFromError`, + ); + return {error}; + } + render() { + const {children, id, fallbackID} = this.props; + const {error} = this.state; + if (error) { + Scheduler.unstable_yieldValue(`${id} render error`); + return ; + } + Scheduler.unstable_yieldValue(`${id} render success`); + return children || null; + } + } + + class Component extends React.Component { + render() { + const {id} = this.props; + Scheduler.unstable_yieldValue('Component render ' + id); + return id; + } + } + + class LocalBrokenComponentWillUnmount extends React.Component { + componentWillUnmount() { + Scheduler.unstable_yieldValue( + 'BrokenComponentWillUnmount componentWillUnmount', + ); + throw Error('Expected'); + } + + render() { + Scheduler.unstable_yieldValue('BrokenComponentWillUnmount render'); + return 'broken'; + } + } + + const container = document.createElement('div'); + + ReactDOM.render( + + + + + + , + container, + ); + + expect(container.firstChild.textContent).toBe('sibling'); + expect(container.lastChild.textContent).toBe('broken'); + expect(Scheduler).toHaveYielded([ + 'OuterBoundary render success', + 'Component render sibling', + 'InnerBoundary render success', + 'BrokenComponentWillUnmount render', + ]); + + ReactDOM.render( + + + , + container, + ); + + // React should skip over the unmounting boundary and find the nearest still-mounted boundary. + expect(container.firstChild.textContent).toBe('OuterFallback'); + expect(container.lastChild.textContent).toBe('OuterFallback'); + expect(Scheduler).toHaveYielded([ + 'OuterBoundary render success', + 'Component render sibling', + 'BrokenComponentWillUnmount componentWillUnmount', + 'ErrorBoundary static getDerivedStateFromError', + 'OuterBoundary render error', + 'Component render OuterFallback', + ]); + }); + + // @gate skipUnmountedBoundaries + it('catches errors thrown while detaching refs', () => { + class LocalErrorBoundary extends React.Component { + state = {error: null}; + static getDerivedStateFromError(error) { + Scheduler.unstable_yieldValue( + `ErrorBoundary static getDerivedStateFromError`, + ); + return {error}; + } + render() { + const {children, id, fallbackID} = this.props; + const {error} = this.state; + if (error) { + Scheduler.unstable_yieldValue(`${id} render error`); + return ; + } + Scheduler.unstable_yieldValue(`${id} render success`); + return children || null; + } + } + + class Component extends React.Component { + render() { + const {id} = this.props; + Scheduler.unstable_yieldValue('Component render ' + id); + return id; + } + } + + class LocalBrokenCallbackRef extends React.Component { + _ref = ref => { + Scheduler.unstable_yieldValue('LocalBrokenCallbackRef ref ' + !!ref); + if (ref === null) { + throw Error('Expected'); + } + }; + + render() { + Scheduler.unstable_yieldValue('LocalBrokenCallbackRef render'); + return
ref
; + } + } + + const container = document.createElement('div'); + + ReactDOM.render( + + + + + + , + container, + ); + + expect(container.firstChild.textContent).toBe('sibling'); + expect(container.lastChild.textContent).toBe('ref'); + expect(Scheduler).toHaveYielded([ + 'OuterBoundary render success', + 'Component render sibling', + 'InnerBoundary render success', + 'LocalBrokenCallbackRef render', + 'LocalBrokenCallbackRef ref true', + ]); + + ReactDOM.render( + + + , + container, + ); + + // React should skip over the unmounting boundary and find the nearest still-mounted boundary. + expect(container.firstChild.textContent).toBe('OuterFallback'); + expect(container.lastChild.textContent).toBe('OuterFallback'); + expect(Scheduler).toHaveYielded([ + 'OuterBoundary render success', + 'Component render sibling', + 'LocalBrokenCallbackRef ref false', + 'ErrorBoundary static getDerivedStateFromError', + 'OuterBoundary render error', + 'Component render OuterFallback', + ]); + }); }); diff --git a/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js b/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js index c80a05c2113b..8e6d1eaa594e 100644 --- a/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js @@ -167,7 +167,7 @@ describe('ReactFunctionComponent', () => { '1. You may be adding a ref to a function component\n' + "2. You may be adding a ref to a component that was not created inside a component's render method\n" + '3. You have multiple copies of React loaded\n' + - 'See https://fb.me/react-refs-must-have-owner for more information.', + 'See https://reactjs.org/link/refs-must-have-owner for more information.', ); }); diff --git a/packages/react-dom/src/__tests__/ReactTestUtils-test.js b/packages/react-dom/src/__tests__/ReactTestUtils-test.js index 20940d7523e5..8b1bccac4416 100644 --- a/packages/react-dom/src/__tests__/ReactTestUtils-test.js +++ b/packages/react-dom/src/__tests__/ReactTestUtils-test.js @@ -46,7 +46,7 @@ describe('ReactTestUtils', () => { ).toWarnDev( 'ReactTestUtils.mockComponent() is deprecated. ' + 'Use shallow rendering or jest.mock() instead.\n\n' + - 'See https://fb.me/test-utils-mock-component for more information.', + 'See https://reactjs.org/link/test-utils-mock-component for more information.', {withoutStack: true}, ); diff --git a/packages/react-dom/src/__tests__/findDOMNode-test.js b/packages/react-dom/src/__tests__/findDOMNode-test.js index 9aaa65088e6c..c634886e8f8d 100644 --- a/packages/react-dom/src/__tests__/findDOMNode-test.js +++ b/packages/react-dom/src/__tests__/findDOMNode-test.js @@ -122,7 +122,7 @@ describe('findDOMNode', () => { 'findDOMNode was passed an instance of ContainsStrictModeChild which renders StrictMode children. ' + 'Instead, add a ref directly to the element you want to reference. ' + 'Learn more about using refs safely here: ' + - 'https://fb.me/react-strict-mode-find-node' + + 'https://reactjs.org/link/strict-mode-find-node' + '\n in div (at **)' + '\n in ContainsStrictModeChild (at **)', ]); @@ -151,7 +151,7 @@ describe('findDOMNode', () => { 'findDOMNode was passed an instance of IsInStrictMode which is inside StrictMode. ' + 'Instead, add a ref directly to the element you want to reference. ' + 'Learn more about using refs safely here: ' + - 'https://fb.me/react-strict-mode-find-node' + + 'https://reactjs.org/link/strict-mode-find-node' + '\n in div (at **)' + '\n in IsInStrictMode (at **)', ]); diff --git a/packages/react-dom/src/__tests__/multiple-copies-of-react-test.js b/packages/react-dom/src/__tests__/multiple-copies-of-react-test.js index 280af615cdf7..5e2d75b65708 100644 --- a/packages/react-dom/src/__tests__/multiple-copies-of-react-test.js +++ b/packages/react-dom/src/__tests__/multiple-copies-of-react-test.js @@ -30,7 +30,7 @@ describe('when different React version is used with string ref', () => { '1. You may be adding a ref to a function component\n' + "2. You may be adding a ref to a component that was not created inside a component's render method\n" + '3. You have multiple copies of React loaded\n' + - 'See https://fb.me/react-refs-must-have-owner for more information.', + 'See https://reactjs.org/link/refs-must-have-owner for more information.', ); }); }); diff --git a/packages/react-dom/src/__tests__/refs-test.js b/packages/react-dom/src/__tests__/refs-test.js index 972e454e398c..4b616c1ae4cf 100644 --- a/packages/react-dom/src/__tests__/refs-test.js +++ b/packages/react-dom/src/__tests__/refs-test.js @@ -458,7 +458,7 @@ describe('creating element with ref in constructor', () => { '1. You may be adding a ref to a function component\n' + "2. You may be adding a ref to a component that was not created inside a component's render method\n" + '3. You have multiple copies of React loaded\n' + - 'See https://fb.me/react-refs-must-have-owner for more information.', + 'See https://reactjs.org/link/refs-must-have-owner for more information.', ); }); }); diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js index a0861bc5d6a0..5daeef818161 100644 --- a/packages/react-dom/src/client/ReactDOM.js +++ b/packages/react-dom/src/client/ReactDOM.js @@ -94,7 +94,7 @@ if (__DEV__) { ) { console.error( 'React depends on Map and Set built-in types. Make sure that you load a ' + - 'polyfill in older browsers. https://fb.me/react-polyfills', + 'polyfill in older browsers. https://reactjs.org/link/react-polyfills', ); } } @@ -238,10 +238,10 @@ if (__DEV__) { console.info( '%cDownload the React DevTools ' + 'for a better development experience: ' + - 'https://fb.me/react-devtools' + + 'https://reactjs.org/link/react-devtools' + (protocol === 'file:' ? '\nYou might need to use a local HTTP server (instead of file://): ' + - 'https://fb.me/react-devtools-faq' + 'https://reactjs.org/link/react-devtools-faq' : ''), 'font-weight:bold', ); diff --git a/packages/react-dom/src/client/ReactDOMComponent.js b/packages/react-dom/src/client/ReactDOMComponent.js index b82a41eb0faa..d3e72d2a2d21 100644 --- a/packages/react-dom/src/client/ReactDOMComponent.js +++ b/packages/react-dom/src/client/ReactDOMComponent.js @@ -13,7 +13,6 @@ import { } from '../events/EventRegistry'; import {canUseDOM} from 'shared/ExecutionEnvironment'; -import invariant from 'shared/invariant'; import { getValueForAttribute, @@ -66,6 +65,7 @@ import { DOCUMENT_NODE, ELEMENT_NODE, COMMENT_NODE, + DOCUMENT_FRAGMENT_NODE, } from '../shared/HTMLNodeType'; import isCustomComponent from '../shared/isCustomComponent'; import possibleStandardNames from '../shared/possibleStandardNames'; @@ -266,15 +266,19 @@ export function ensureListeningTo( rootContainerInstance.nodeType === COMMENT_NODE ? rootContainerInstance.parentNode : rootContainerInstance; - // Containers should only ever be element nodes. We do not - // want to register events to document fragments or documents - // with the modern plugin event system. - invariant( - rootContainerElement != null && - rootContainerElement.nodeType === ELEMENT_NODE, - 'ensureListeningTo(): received a container that was not an element node. ' + - 'This is likely a bug in React.', - ); + if (__DEV__) { + if ( + rootContainerElement == null || + (rootContainerElement.nodeType !== ELEMENT_NODE && + // This is to support rendering into a ShadowRoot: + rootContainerElement.nodeType !== DOCUMENT_FRAGMENT_NODE) + ) { + console.error( + 'ensureListeningTo(): received a container that was not an element node. ' + + 'This is likely a bug in React. Please file an issue.', + ); + } + } listenToReactEvent( reactPropEvent, ((rootContainerElement: any): Element), diff --git a/packages/react-dom/src/client/ReactDOMInput.js b/packages/react-dom/src/client/ReactDOMInput.js index e503b75a9546..3bfdd8b01f64 100644 --- a/packages/react-dom/src/client/ReactDOMInput.js +++ b/packages/react-dom/src/client/ReactDOMInput.js @@ -86,7 +86,7 @@ export function initWrapperState(element: Element, props: Object) { '(specify either the checked prop, or the defaultChecked prop, but not ' + 'both). Decide between using a controlled or uncontrolled input ' + 'element and remove one of these props. More info: ' + - 'https://fb.me/react-controlled-components', + 'https://reactjs.org/link/controlled-components', getCurrentFiberOwnerNameInDevOrNull() || 'A component', props.type, ); @@ -103,7 +103,7 @@ export function initWrapperState(element: Element, props: Object) { '(specify either the value prop, or the defaultValue prop, but not ' + 'both). Decide between using a controlled or uncontrolled input ' + 'element and remove one of these props. More info: ' + - 'https://fb.me/react-controlled-components', + 'https://reactjs.org/link/controlled-components', getCurrentFiberOwnerNameInDevOrNull() || 'A component', props.type, ); @@ -147,7 +147,7 @@ export function updateWrapper(element: Element, props: Object) { 'This is likely caused by the value changing from undefined to ' + 'a defined value, which should not happen. ' + 'Decide between using a controlled or uncontrolled input ' + - 'element for the lifetime of the component. More info: https://fb.me/react-controlled-components', + 'element for the lifetime of the component. More info: https://reactjs.org/link/controlled-components', ); didWarnUncontrolledToControlled = true; } @@ -161,7 +161,7 @@ export function updateWrapper(element: Element, props: Object) { 'This is likely caused by the value changing from a defined to ' + 'undefined, which should not happen. ' + 'Decide between using a controlled or uncontrolled input ' + - 'element for the lifetime of the component. More info: https://fb.me/react-controlled-components', + 'element for the lifetime of the component. More info: https://reactjs.org/link/controlled-components', ); didWarnControlledToUncontrolled = true; } diff --git a/packages/react-dom/src/client/ReactDOMSelect.js b/packages/react-dom/src/client/ReactDOMSelect.js index d97e01be0ab6..1efca61905b4 100644 --- a/packages/react-dom/src/client/ReactDOMSelect.js +++ b/packages/react-dom/src/client/ReactDOMSelect.js @@ -159,7 +159,7 @@ export function initWrapperState(element: Element, props: Object) { '(specify either the value prop, or the defaultValue prop, but not ' + 'both). Decide between using a controlled or uncontrolled select ' + 'element and remove one of these props. More info: ' + - 'https://fb.me/react-controlled-components', + 'https://reactjs.org/link/controlled-components', ); didWarnValueDefaultValue = true; } diff --git a/packages/react-dom/src/client/ReactDOMTextarea.js b/packages/react-dom/src/client/ReactDOMTextarea.js index a37c36b94efd..24fb591fcd8a 100644 --- a/packages/react-dom/src/client/ReactDOMTextarea.js +++ b/packages/react-dom/src/client/ReactDOMTextarea.js @@ -76,7 +76,7 @@ export function initWrapperState(element: Element, props: Object) { '(specify either the value prop, or the defaultValue prop, but not ' + 'both). Decide between using a controlled or uncontrolled textarea ' + 'and remove one of these props. More info: ' + - 'https://fb.me/react-controlled-components', + 'https://reactjs.org/link/controlled-components', getCurrentFiberOwnerNameInDevOrNull() || 'A component', ); didWarnValDefaultVal = true; diff --git a/packages/react-dom/src/events/DOMPluginEventSystem.js b/packages/react-dom/src/events/DOMPluginEventSystem.js index 1b23cde2f35a..6a216ace127c 100644 --- a/packages/react-dom/src/events/DOMPluginEventSystem.js +++ b/packages/react-dom/src/events/DOMPluginEventSystem.js @@ -15,7 +15,10 @@ import { SHOULD_NOT_PROCESS_POLYFILL_EVENT_PLUGINS, } from './EventSystemFlags'; import type {AnyNativeEvent} from './PluginModuleType'; -import type {ReactSyntheticEvent} from './ReactSyntheticEventType'; +import type { + KnownReactSyntheticEvent, + ReactSyntheticEvent, +} from './ReactSyntheticEventType'; import type {ElementListenerMapEntry} from '../client/ReactDOMComponentTree'; import type {EventPriority} from 'shared/ReactTypes'; import type {Fiber} from 'react-reconciler/src/ReactInternalTypes'; @@ -50,6 +53,7 @@ import { enableLegacyFBSupport, enableCreateEventHandleAPI, enableScopeAPI, + enablePassiveEventIntervention, } from 'shared/ReactFeatureFlags'; import { invokeGuardedCallbackAndCatchFirstError, @@ -339,6 +343,21 @@ export function listenToNativeEvent( if (domEventName === 'selectionchange') { target = (rootContainerElement: any).ownerDocument; } + if (enablePassiveEventIntervention && isPassiveListener === undefined) { + // Browsers introduced an intervention, making these events + // passive by default on document. React doesn't bind them + // to document anymore, but changing this now would undo + // the performance wins from the change. So we emulate + // the existing behavior manually on the roots now. + // https://github.com/facebook/react/issues/19651 + if ( + domEventName === 'touchstart' || + domEventName === 'touchmove' || + domEventName === 'wheel' + ) { + isPassiveListener = true; + } + } // If the event can be delegated (or is capture phase), we can // register it to the root container. Otherwise, we should // register the event to the target element and mark it as @@ -503,7 +522,7 @@ function addTrappedEventListener( }; } if (isCapturePhaseListener) { - if (enableCreateEventHandleAPI && isPassiveListener !== undefined) { + if (isPassiveListener !== undefined) { unsubscribeListener = addEventCaptureListenerWithPassiveFlag( targetContainer, domEventName, @@ -518,7 +537,7 @@ function addTrappedEventListener( ); } } else { - if (enableCreateEventHandleAPI && isPassiveListener !== undefined) { + if (isPassiveListener !== undefined) { unsubscribeListener = addEventBubbleListenerWithPassiveFlag( targetContainer, domEventName, @@ -709,7 +728,7 @@ export function accumulateSinglePhaseListeners( let instance = targetFiber; let lastHostComponent = null; - const targetType = event.type; + const targetType = event.nativeEvent.type; // Accumulate all instances and listeners via the target -> root path. while (instance !== null) { @@ -917,15 +936,12 @@ function getLowestCommonAncestor(instA: Fiber, instB: Fiber): Fiber | null { function accumulateEnterLeaveListenersForEvent( dispatchQueue: DispatchQueue, - event: ReactSyntheticEvent, + event: KnownReactSyntheticEvent, target: Fiber, common: Fiber | null, inCapturePhase: boolean, ): void { const registrationName = event._reactName; - if (registrationName === undefined) { - return; - } const listeners: Array = []; let instance = target; @@ -969,8 +985,8 @@ function accumulateEnterLeaveListenersForEvent( // phase event listeners. export function accumulateEnterLeaveTwoPhaseListeners( dispatchQueue: DispatchQueue, - leaveEvent: ReactSyntheticEvent, - enterEvent: null | ReactSyntheticEvent, + leaveEvent: KnownReactSyntheticEvent, + enterEvent: null | KnownReactSyntheticEvent, from: Fiber | null, to: Fiber | null, ): void { diff --git a/packages/react-dom/src/events/ReactSyntheticEventType.js b/packages/react-dom/src/events/ReactSyntheticEventType.js index f68fad1e0b6d..853aa3a6aefa 100644 --- a/packages/react-dom/src/events/ReactSyntheticEventType.js +++ b/packages/react-dom/src/events/ReactSyntheticEventType.js @@ -22,13 +22,26 @@ export type DispatchConfig = {| eventPriority?: EventPriority, |}; -export type ReactSyntheticEvent = {| +type BaseSyntheticEvent = { isPersistent: () => boolean, isPropagationStopped: () => boolean, _dispatchInstances?: null | Array | Fiber, _dispatchListeners?: null | Array | Function, - _reactName: string, _targetInst: Fiber, + nativeEvent: Event, + target?: mixed, + relatedTarget?: mixed, type: string, currentTarget: null | EventTarget, -|}; +}; + +export type KnownReactSyntheticEvent = BaseSyntheticEvent & { + _reactName: string, +}; +export type UnknownReactSyntheticEvent = BaseSyntheticEvent & { + _reactName: null, +}; + +export type ReactSyntheticEvent = + | KnownReactSyntheticEvent + | UnknownReactSyntheticEvent; diff --git a/packages/react-dom/src/events/SyntheticEvent.js b/packages/react-dom/src/events/SyntheticEvent.js index 3edfae5f853f..01742c705278 100644 --- a/packages/react-dom/src/events/SyntheticEvent.js +++ b/packages/react-dom/src/events/SyntheticEvent.js @@ -3,18 +3,23 @@ * * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. + * + * @flow */ /* eslint valid-typeof: 0 */ import getEventCharCode from './getEventCharCode'; +type EventInterfaceType = { + [propName: string]: 0 | ((event: {[propName: string]: mixed}) => mixed), +}; + /** * @interface Event * @see http://www.w3.org/TR/DOM-Level-3-Events/ */ -const EventInterface = { - type: 0, +const EventInterface: EventInterfaceType = { eventPhase: 0, bubbles: 0, cancelable: 0, @@ -47,14 +52,16 @@ function functionThatReturnsFalse() { * DOM interface; custom application-specific events can also subclass this. */ export function SyntheticEvent( - reactName, - targetInst, - nativeEvent, - nativeEventTarget, - Interface = EventInterface, + reactName: string | null, + reactEventType: string, + targetInst: Fiber, + nativeEvent: {[propName: string]: mixed}, + nativeEventTarget: null | EventTarget, + Interface: EventInterfaceType = EventInterface, ) { this._reactName = reactName; this._targetInst = targetInst; + this.type = reactEventType; this.nativeEvent = nativeEvent; this.target = nativeEventTarget; this.currentTarget = null; @@ -94,6 +101,7 @@ Object.assign(SyntheticEvent.prototype, { if (event.preventDefault) { event.preventDefault(); + // $FlowFixMe - flow is not aware of `unknown` in IE } else if (typeof event.returnValue !== 'unknown') { event.returnValue = false; } @@ -108,6 +116,7 @@ Object.assign(SyntheticEvent.prototype, { if (event.stopPropagation) { event.stopPropagation(); + // $FlowFixMe - flow is not aware of `unknown` in IE } else if (typeof event.cancelBubble !== 'unknown') { // The ChangeEventPlugin registers a "propertychange" event for // IE. This event does not support bubbling or cancelling, and @@ -137,7 +146,7 @@ Object.assign(SyntheticEvent.prototype, { isPersistent: functionThatReturnsTrue, }); -export const UIEventInterface = { +export const UIEventInterface: EventInterfaceType = { ...EventInterface, view: 0, detail: 0, @@ -153,7 +162,7 @@ let isMovementYSet = false; * @interface MouseEvent * @see http://www.w3.org/TR/DOM-Level-3-Events/ */ -export const MouseEventInterface = { +export const MouseEventInterface: EventInterfaceType = { ...UIEventInterface, screenX: 0, screenY: 0, @@ -169,12 +178,12 @@ export const MouseEventInterface = { button: 0, buttons: 0, relatedTarget: function(event) { - return ( - event.relatedTarget || - (event.fromElement === event.srcElement + if (event.relatedTarget === undefined) + return event.fromElement === event.srcElement ? event.toElement - : event.fromElement) - ); + : event.fromElement; + + return event.relatedTarget; }, movementX: function(event) { if ('movementX' in event) { @@ -212,7 +221,7 @@ export const MouseEventInterface = { * @interface DragEvent * @see http://www.w3.org/TR/DOM-Level-3-Events/ */ -export const DragEventInterface = { +export const DragEventInterface: EventInterfaceType = { ...MouseEventInterface, dataTransfer: 0, }; @@ -221,7 +230,7 @@ export const DragEventInterface = { * @interface FocusEvent * @see http://www.w3.org/TR/DOM-Level-3-Events/ */ -export const FocusEventInterface = { +export const FocusEventInterface: EventInterfaceType = { ...UIEventInterface, relatedTarget: 0, }; @@ -231,7 +240,7 @@ export const FocusEventInterface = { * @see http://www.w3.org/TR/css3-animations/#AnimationEvent-interface * @see https://developer.mozilla.org/en-US/docs/Web/API/AnimationEvent */ -export const AnimationEventInterface = { +export const AnimationEventInterface: EventInterfaceType = { ...EventInterface, animationName: 0, elapsedTime: 0, @@ -242,7 +251,7 @@ export const AnimationEventInterface = { * @interface Event * @see http://www.w3.org/TR/clipboard-apis/ */ -export const ClipboardEventInterface = { +export const ClipboardEventInterface: EventInterfaceType = { ...EventInterface, clipboardData: function(event) { return 'clipboardData' in event @@ -255,7 +264,7 @@ export const ClipboardEventInterface = { * @interface Event * @see http://www.w3.org/TR/DOM-Level-3-Events/#events-compositionevents */ -export const CompositionEventInterface = { +export const CompositionEventInterface: EventInterfaceType = { ...EventInterface, data: 0, }; @@ -266,7 +275,7 @@ export const CompositionEventInterface = { * /#events-inputevents */ // Happens to share the same list for now. -export const InputEventInterface = CompositionEventInterface; +export const InputEventInterface: EventInterfaceType = CompositionEventInterface; /** * Normalization of deprecated HTML5 `key` values diff --git a/packages/react-dom/src/events/__tests__/DOMPluginEventSystem-test.internal.js b/packages/react-dom/src/events/__tests__/DOMPluginEventSystem-test.internal.js index 794f48ce676f..8858997cb97d 100644 --- a/packages/react-dom/src/events/__tests__/DOMPluginEventSystem-test.internal.js +++ b/packages/react-dom/src/events/__tests__/DOMPluginEventSystem-test.internal.js @@ -1667,16 +1667,28 @@ describe('DOMPluginEventSystem', () => { function Test() { React.useEffect(() => { - setClick1(buttonRef.current, targetListener1); - setClick2(buttonRef.current, targetListener2); - setClick3(buttonRef.current, targetListener3); - setClick4(buttonRef.current, targetListener4); + const clearClick1 = setClick1( + buttonRef.current, + targetListener1, + ); + const clearClick2 = setClick2( + buttonRef.current, + targetListener2, + ); + const clearClick3 = setClick3( + buttonRef.current, + targetListener3, + ); + const clearClick4 = setClick4( + buttonRef.current, + targetListener4, + ); return () => { - setClick1(); - setClick2(); - setClick3(); - setClick4(); + clearClick1(); + clearClick2(); + clearClick3(); + clearClick4(); }; }); @@ -1701,16 +1713,28 @@ describe('DOMPluginEventSystem', () => { function Test2() { React.useEffect(() => { - setClick1(buttonRef.current, targetListener1); - setClick2(buttonRef.current, targetListener2); - setClick3(buttonRef.current, targetListener3); - setClick4(buttonRef.current, targetListener4); + const clearClick1 = setClick1( + buttonRef.current, + targetListener1, + ); + const clearClick2 = setClick2( + buttonRef.current, + targetListener2, + ); + const clearClick3 = setClick3( + buttonRef.current, + targetListener3, + ); + const clearClick4 = setClick4( + buttonRef.current, + targetListener4, + ); return () => { - setClick1(); - setClick2(); - setClick3(); - setClick4(); + clearClick1(); + clearClick2(); + clearClick3(); + clearClick4(); }; }); diff --git a/packages/react-dom/src/events/__tests__/SyntheticFocusEvent-test.js b/packages/react-dom/src/events/__tests__/SyntheticFocusEvent-test.js new file mode 100644 index 000000000000..1b63e5c3b665 --- /dev/null +++ b/packages/react-dom/src/events/__tests__/SyntheticFocusEvent-test.js @@ -0,0 +1,70 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +'use strict'; + +describe('SyntheticFocusEvent', () => { + let React; + let ReactDOM; + let container; + + beforeEach(() => { + jest.resetModules(); + React = require('react'); + ReactDOM = require('react-dom'); + + container = document.createElement('div'); + document.body.appendChild(container); + }); + + afterEach(() => { + document.body.removeChild(container); + container = null; + }); + + test('onFocus events have the focus type', () => { + const log = []; + ReactDOM.render( +