Skip to content

Commit

Permalink
[Flight] Instrument the Promise for Async Module instead of using a M…
Browse files Browse the repository at this point in the history
…odule Cache (#26985)

Currently, since we use a module cache for async modules, it doesn't
automatically get updated when the module registry gets updated (HMR).

This technique ensures that if Webpack replaces the module (HMR) then
we'll get the new Promise when we require it again.

This technique doesn't work for ESM and probably not Vite since ESM will
provide a new Promise each time you call `import()` but in the
Webpack/CJS approach this Promise is an entry in the module cache and
not a promise for the entry.

I tried to replicate the original issue in the fixture but it's tricky
to replicate because 1) we can't really use async modules the same way
without compiling both server and client 2) even then I'm not quite sure
how to repro the HMR issue.
  • Loading branch information
sebmarkbage committed Jun 28, 2023
1 parent a1c62b8 commit 5945e06
Show file tree
Hide file tree
Showing 13 changed files with 1,154 additions and 1,375 deletions.
29 changes: 0 additions & 29 deletions fixtures/flight/config/jest/babelTransform.js

This file was deleted.

14 changes: 0 additions & 14 deletions fixtures/flight/config/jest/cssTransform.js

This file was deleted.

40 changes: 0 additions & 40 deletions fixtures/flight/config/jest/fileTransform.js

This file was deleted.

32 changes: 3 additions & 29 deletions fixtures/flight/config/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ const MiniCssExtractPlugin = require('mini-css-extract-plugin');
const CssMinimizerPlugin = require('css-minimizer-webpack-plugin');
const ModuleScopePlugin = require('react-dev-utils/ModuleScopePlugin');
const getCSSModuleLocalIdent = require('react-dev-utils/getCSSModuleLocalIdent');
const ESLintPlugin = require('eslint-webpack-plugin');
const paths = require('./paths');
const modules = require('./modules');
const getClientEnvironment = require('./env');
Expand Down Expand Up @@ -54,9 +53,6 @@ const babelRuntimeRegenerator = require.resolve('@babel/runtime/regenerator', {
// makes for a smoother build process.
const shouldInlineRuntimeChunk = process.env.INLINE_RUNTIME_CHUNK !== 'false';

const emitErrorsAsWarnings = process.env.ESLINT_NO_DEV_ERRORS === 'true';
const disableESLintPlugin = process.env.DISABLE_ESLINT_PLUGIN === 'true';

const imageInlineSizeLimit = parseInt(
process.env.IMAGE_INLINE_SIZE_LIMIT || '10000'
);
Expand Down Expand Up @@ -687,31 +683,6 @@ module.exports = function (webpackEnv) {
infrastructure: 'silent',
},
}),
// !disableESLintPlugin &&
// new ESLintPlugin({
// // Plugin options
// extensions: ['js', 'mjs', 'jsx', 'ts', 'tsx'],
// formatter: require.resolve('react-dev-utils/eslintFormatter'),
// eslintPath: require.resolve('eslint'),
// failOnError: !(isEnvDevelopment && emitErrorsAsWarnings),
// context: paths.appSrc,
// cache: true,
// cacheLocation: path.resolve(
// paths.appNodeModules,
// '.cache/.eslintcache'
// ),
// // ESLint class options
// cwd: paths.appPath,
// resolvePluginsRelativeTo: __dirname,
// baseConfig: {
// extends: [require.resolve('eslint-config-react-app/base')],
// rules: {
// ...(!hasJsxRuntime && {
// 'react/react-in-jsx-scope': 'error',
// }),
// },
// },
// }),
// Fork Start
new ReactFlightWebpackPlugin({
isServer: false,
Expand All @@ -723,6 +694,9 @@ module.exports = function (webpackEnv) {
}),
// Fork End
].filter(Boolean),
experiments: {
topLevelAwait: true,
},
// Turn off performance processing because we utilize
// our own hints via the FileSizeReporter
performance: false,
Expand Down
11 changes: 1 addition & 10 deletions fixtures/flight/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"dependencies": {
"@babel/core": "^7.16.0",
"@babel/plugin-proposal-private-property-in-object": "^7.18.6",
"@babel/preset-react": "^7.22.5",
"@pmmmwh/react-refresh-webpack-plugin": "^0.5.3",
"@svgr/webpack": "^5.5.0",
"@testing-library/jest-dom": "^5.14.1",
Expand All @@ -26,9 +27,6 @@
"css-minimizer-webpack-plugin": "^3.2.0",
"dotenv": "^10.0.0",
"dotenv-expand": "^5.1.0",
"eslint": "^8.3.0",
"eslint-config-react-app": "^7.0.1",
"eslint-webpack-plugin": "^3.1.1",
"file-loader": "^6.2.0",
"fs-extra": "^10.0.0",
"html-webpack-plugin": "^5.5.0",
Expand All @@ -45,7 +43,6 @@
"postcss-preset-env": "^7.0.1",
"prompts": "^2.4.2",
"react": "experimental",
"react-app-polyfill": "^3.0.0",
"react-dev-utils": "^12.0.1",
"react-dom": "experimental",
"react-refresh": "^0.11.0",
Expand Down Expand Up @@ -75,12 +72,6 @@
"build": "node scripts/build.js",
"test": "node scripts/test.js --env=jsdom"
},
"eslintConfig": {
"extends": [
"react-app",
"react-app/jest"
]
},
"browserslist": {
"production": [
">0.2%",
Expand Down
6 changes: 1 addition & 5 deletions fixtures/flight/server/global.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@ const path = require('path');
// Do this as the first thing so that any code reading it knows the right env.
process.env.BABEL_ENV = process.env.NODE_ENV;

const register = require('react-server-dom-webpack/node-register');
register();

const babelRegister = require('@babel/register');
babelRegister({
babelrc: false,
Expand All @@ -25,8 +22,7 @@ babelRegister({
return false;
},
],
presets: ['react-app'],
plugins: ['@babel/transform-modules-commonjs'],
presets: ['@babel/preset-react'],
});

// Ensure environment variables are read.
Expand Down
2 changes: 1 addition & 1 deletion fixtures/flight/server/region.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ babelRegister({
return false;
},
],
presets: ['react-app'],
presets: ['@babel/preset-react'],
plugins: ['@babel/transform-modules-commonjs'],
});

Expand Down
3 changes: 3 additions & 0 deletions fixtures/flight/src/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import Container from './Container.js';

import {Counter} from './Counter.js';
import {Counter as Counter2} from './Counter2.js';
import AsyncModule from './cjs/Counter3.js';
const Counter3 = await(AsyncModule);

import ShowMore from './ShowMore.js';
import Button from './Button.js';
Expand All @@ -28,6 +30,7 @@ export default async function App() {
<h1>{getServerState()}</h1>
<Counter />
<Counter2 />
<Counter3 />
<ul>
{todos.map(todo => (
<li key={todo.id}>{todo.text}</li>
Expand Down
5 changes: 5 additions & 0 deletions fixtures/flight/src/cjs/Counter3.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
"use client";
// CJS-ESM async module
module.exports = import('../Counter.js').then(m => {
return m.Counter
});
3 changes: 3 additions & 0 deletions fixtures/flight/src/cjs/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"type":"commonjs"
}
Loading

0 comments on commit 5945e06

Please sign in to comment.