Merge #2070

2070: Fix `importComponent` r=ZauberNerd a=ZauberNerd Ref: #1632, #1976 Todo: - [x] What happens if we import the same file from different files? I suspect our babel plugin will generate different chunk names which should in fact not be different. So maybe our hash should not include the filepath of the importing component but only the filepath of the imported component. - seems to create separate entries in the `namedChunkGroups` which point to the same asset. <details> <summary>Bors merge bot cheat sheet</summary> We are using [bors-ng](https://github.com/bors-ng/bors-ng) to automate merging of our pull requests. The following table provides a summary of commands that are available to reviewers (members of this repository with push access) and delegates (in case of `bors delegate+` or `bors delegate=[list]`). | Syntax | Description | | --- | --- | | bors merge | Run the test suite and push to master if it passes. Short for "reviewed: looks good." | | bors merge- | Cancel an r+, r=, merge, or merge= | | bors try | Run the test suite without pushing to master. | | bors try- | Cancel a try | | bors delegate+ | Allow the pull request author to merge their changes. | | bors delegate=[list] | Allow the listed users to r+ this pull request's changes. | | bors retry | Run the previous command a second time. | This is a short collection of opinionated commands. For a full list of the commands read the [bors reference](https://bors.tech/documentation/). </details> Co-authored-by: Björn Brauer <bjoern.brauer@new-work.se> Co-authored-by: Robert Kowalski <robert.kowalski@new-work.se>
xing · Feb 16, 2022 · 44b8899 · 44b8899
2 parents 9f97080 + b134446
commit 44b8899
Show file tree

Hide file tree

Showing 36 changed files with 298 additions and 125 deletions.
diff --git a/package.json b/package.json
@@ -54,11 +54,13 @@
           },
           {
             "url": "enc:r6BTklZr+axySHpOBj0JysOusV4S8o7YtSxZ9fi66rL5grn3vMkHUbMdb3OY5+tv",
-            "directory": "internal-hops"
+            "directory": "internal-hops",
+            "branch": "wait-for-compilation"
           }
         ],
         "rootManifest": {
           "resolutions": {
+            "@babel/core": "7.15.0",
             "colors": "1.4.0",
             "graphql-tag": "^2.12"
           }
@@ -250,7 +252,7 @@
     "@commitlint/cli": "^13.0.0",
     "@commitlint/config-conventional": "^13.0.0",
     "@commitlint/config-lerna-scopes": "^13.0.0",
-    "canarist": "^2.3.1",
+    "canarist": "^2.3.2",
     "cz-conventional-changelog": "^3.3.0",
     "eslint": "^8.0.0",
     "eslint-config-prettier": "^8.3.0",

diff --git a/packages/jest-environment/helpers.js b/packages/jest-environment/helpers.js
@@ -45,6 +45,8 @@ const build = ({ cwd, env = {}, argv = [] }) =>
 const startServer = ({ cwd, command, env = {}, argv = [] }) => {
   const teardownPromise = new EProm();
   const urlPromise = new EProm();
+  const hasFinishedPromise = new EProm();
+
   let stdout = '';
   let stderr = '';
 
@@ -61,6 +63,12 @@ const startServer = ({ cwd, command, env = {}, argv = [] }) => {
     return teardownPromise;
   };
 
+  let linesToFinish = [];
+  const hasFinished = (lines) => {
+    linesToFinish.push(...lines);
+    return hasFinishedPromise;
+  };
+
   started.stdout.on('data', (data) => {
     const line = data.toString('utf-8');
     debug('stdout >', line);
@@ -71,6 +79,19 @@ const startServer = ({ cwd, command, env = {}, argv = [] }) => {
       debug('found match:', url);
       urlPromise.resolve(url);
     }
+
+    if (
+      linesToFinish.length > 0 &&
+      linesToFinish.every((lineToFinish) => stdout.includes(lineToFinish))
+    ) {
+      hasFinishedPromise.resolve();
+    }
+
+    if (line.match(/bundling [^ ]+ failed/)) {
+      setTimeout(() => {
+        hasFinishedPromise.reject({ stdout, stderr });
+      }, 5000);
+    }
   });
 
   started.stderr.on('data', (data) => {
@@ -91,7 +112,7 @@ const startServer = ({ cwd, command, env = {}, argv = [] }) => {
     stopServer();
   });
 
-  return { getUrl: () => urlPromise, stopServer };
+  return { getUrl: () => urlPromise, stopServer, hasFinished };
 };
 
 const isPuppeteerDisabled = (pragmas) => {

diff --git a/packages/jest-environment/index.js b/packages/jest-environment/index.js
@@ -112,7 +112,11 @@ class FixtureEnvironment extends NodeEnvironment {
           );
         }
         const { env, argv } = getHopsCommandModifications(args);
-        const { getUrl, stopServer: killServer } = startServer({
+        const {
+          getUrl,
+          stopServer: killServer,
+          hasFinished,
+        } = startServer({
           cwd: that.cwd,
           command: 'start',
           env,
@@ -124,7 +128,7 @@ class FixtureEnvironment extends NodeEnvironment {
           delete that.killServer;
           return killServer();
         };
-        return { getUrl, stopServer };
+        return { getUrl, stopServer, hasFinished };
       },
     };
   }

diff --git a/packages/jest-preset/package.json b/packages/jest-preset/package.json
@@ -24,6 +24,7 @@
   "dependencies": {
     "@babel/core": "^7.9.0",
     "@babel/plugin-proposal-class-properties": "^7.8.3",
+    "@babel/plugin-syntax-jsx": "^7.8.3",
     "@babel/plugin-transform-flow-strip-types": "^7.9.0",
     "@babel/preset-env": "^7.9.5",
     "@babel/preset-react": "^7.9.4",

diff --git a/packages/jest-preset/transforms/esbuild.js b/packages/jest-preset/transforms/esbuild.js
@@ -1,17 +1,27 @@
 // eslint-disable-next-line node/no-extraneous-require
 const { createTransformer } = require('esbuild-jest');
+const { transformSync } = require('@babel/core');
 
 const transformer = createTransformer({
   sourcemap: true,
 });
 
+const regex = /importComponent/g;
+
 module.exports = {
   process(content, filename, config, opts) {
-    content = content.replace(
-      /importComponent\s*\(\s*\(\)\s+=>\s+import\(\s*'([^']+)'\s*\)\s*\)/g,
-      "importComponent({ component: require('$1') })"
-    );
+    if (!regex.test(content)) {
+      return transformer.process(content, filename, config, opts);
+    }
+
+    const result = transformSync(content, {
+      plugins: [
+        require.resolve('../helpers/babel-plugin-import-component'),
+        require.resolve('@babel/plugin-syntax-jsx'),
+      ],
+      filename,
+    });
 
-    return transformer.process(content, filename, config, opts);
+    return transformer.process(result.code, filename, config, opts);
   },
 };
diff --git a/packages/react/import-component/babel.js b/packages/react/import-component/babel.js
@@ -1,8 +1,49 @@
 'use strict';
 
+const nodePath = require('path');
+const crypto = require('crypto');
+
+/**
+ * This plugin transforms the following function call:
+ * ```javascript
+ * importComponent(() => import('./component'));
+ * ```
+ *
+ * Into the following:
+ *
+ * ```javascript
+ * importComponent({
+ *  load: () => import('./component'),
+ *  moduleId: require.resolveWeak('./component'),
+ *  chunkName: () => 'hash'
+ * })
+ * ```
+ */
+
+const regex = /webpackChunkName:\s*(?:(['"])([^'"]*)\1|([^\s]*))/;
+
+function extractChunkName(t, leadingComments) {
+  if (!leadingComments || !Array.isArray(leadingComments)) {
+    return;
+  }
+
+  const comment = leadingComments.find((comment) =>
+    comment.value.includes('webpackChunkName')
+  );
+
+  if (!comment) {
+    return;
+  }
+
+  const results = comment.value.match(regex);
+  const cleanChunkName = results[2] || results[3];
+
+  return cleanChunkName;
+}
+
 module.exports = ({ types: t }) => ({
   visitor: {
-    ImportDeclaration(path) {
+    ImportDeclaration(path, state) {
       const modules = ['hops-react', this.opts.module];
       const source = path.node.source.value;
       if (!modules.includes(source)) return;
@@ -31,19 +72,37 @@ module.exports = ({ types: t }) => ({
         t.assertCallExpression(argument.get('body'));
         t.assertImport(argument.get('body.callee'));
 
-        const { node } = argument.get('body.arguments.0');
-        const importedComponent = node.value;
-        const leadingComments = node.leadingComments;
+        const argPath = argument.get('body.arguments.0');
+        const importedComponent = argPath.node.value;
         const resourcePathNode = t.stringLiteral(importedComponent);
 
+        let chunkName = extractChunkName(t, argPath.node.leadingComments);
+        if (!chunkName) {
+          const hasher = crypto.createHash('md4');
+          hasher.update(nodePath.relative(this.opts.rootDir, state.filename));
+          hasher.update(importedComponent);
+          const hash = hasher.digest('base64').slice(0, 4);
+
+          t.addComment(
+            argPath.node,
+            'leading',
+            ` webpackChunkName: '${hash}' `
+          );
+          chunkName = hash;
+        }
+
         argument.replaceWith(
           t.objectExpression([
             t.objectProperty(
               t.identifier('load'),
               t.arrowFunctionExpression(
                 [],
                 t.callExpression(t.identifier('import'), [
-                  t.addComments(resourcePathNode, 'leading', leadingComments),
+                  t.addComments(
+                    resourcePathNode,
+                    'leading',
+                    argPath.node.leadingComments
+                  ),
                 ])
               )
             ),
@@ -57,6 +116,10 @@ module.exports = ({ types: t }) => ({
                 [resourcePathNode]
               )
             ),
+            t.objectProperty(
+              t.identifier('chunkName'),
+              t.arrowFunctionExpression([], t.stringLiteral(chunkName))
+            ),
           ])
         );
       });

diff --git a/packages/react/import-component/import-component-loader.js b/packages/react/import-component/import-component-loader.js
@@ -1,10 +1,31 @@
-const regex =
-  /importComponent\s*\(\s*\(\)\s+=>\s+import\(\s*'([^']+)'\s*\)\s*\)/g;
+const { transform } = require('@babel/core');
+const regex = /importComponent/g;
 
 function importComponentLoader(source) {
-  return source.replace(
-    regex,
-    "importComponent({ load: () => import('$1'), moduleId: require.resolveWeak('$1') })"
+  const callback = this.async();
+
+  if (!regex.test(source)) {
+    return callback(null, source);
+  }
+
+  const options = this.getOptions();
+
+  transform(
+    source,
+    {
+      plugins: [
+        [require.resolve('./babel'), options],
+        require.resolve('@babel/plugin-syntax-jsx'),
+      ],
+      filename: this.resourcePath,
+    },
+    (err, result) => {
+      if (err) {
+        return callback(err);
+      }
+
+      callback(null, result.code, result.map);
+    }
   );
 }
 

diff --git a/packages/react/import-component/index.js b/packages/react/import-component/index.js
@@ -5,13 +5,13 @@ import PropTypes from 'prop-types';
 import ImportComponentContext from './context';
 
 export const importComponent = (
-  { load, moduleId },
+  { load, moduleId, chunkName },
   resolve = (module) => module.default
 ) => {
   class Importer extends Component {
-    constructor({ hasModules }) {
+    constructor() {
       super();
-      if (hasModules || __webpack_modules__[moduleId]) {
+      if (__webpack_modules__[moduleId]) {
         this.state = { Component: resolve(__webpack_require__(moduleId)) };
       } else {
         this.state = { loading: true };
@@ -57,21 +57,19 @@ export const importComponent = (
   }
 
   Importer.propTypes = {
-    hasModules: PropTypes.bool,
     ownProps: PropTypes.object.isRequired,
     loader: PropTypes.func,
     render: PropTypes.func,
   };
 
   function Import({ loader, render, ...ownProps }) {
-    const modules = useContext(ImportComponentContext);
+    const chunks = useContext(ImportComponentContext);
 
-    if (modules) {
-      modules.push(moduleId);
+    if (chunks) {
+      chunks.push(chunkName());
     }
 
     return createElement(Importer, {
-      hasModules: Boolean(modules && modules.length > 0),
       loader,
       render,
       ownProps,

diff --git a/packages/react/import-component/mixin.core.js b/packages/react/import-component/mixin.core.js
@@ -5,11 +5,14 @@ class ImportComponentCoreMixin extends Mixin {
     const { experimentalEsbuild } = this.options;
 
     if (experimentalEsbuild) {
-      jsLoaderConfig.use.push(require.resolve('./import-component-loader.js'));
+      jsLoaderConfig.use.push({
+        loader: require.resolve('./import-component-loader.js'),
+        options: { module: 'hops', rootDir: this.config.rootDir },
+      });
     } else {
       jsLoaderConfig.options.plugins.push([
         require.resolve('../lib/babel'),
-        { module: 'hops' },
+        { module: 'hops', rootDir: this.config.rootDir },
       ]);
     }
   }

diff --git a/packages/react/import-component/mixin.runtime.js b/packages/react/import-component/mixin.runtime.js
@@ -5,12 +5,12 @@ import { Provider } from './context';
 class ImportComponentMixin extends Mixin {
   bootstrap(_req, res) {
     if (res) {
-      res.locals.modules = this.modules = [];
+      res.locals.chunks = this.chunks = [];
     }
   }
 
   enhanceElement(element) {
-    return createElement(Provider, { value: this.modules }, element);
+    return createElement(Provider, { value: this.chunks }, element);
   }
 }
 

diff --git a/packages/react/lib/assets.js b/packages/react/lib/assets.js
@@ -1,15 +1,19 @@
 'use strict';
 
 import { extname } from 'path';
+import { ensureLeadingSlash } from 'pathifist';
 
-export default (stats, modules) => {
-  const { entryFiles, vendorFiles, moduleFileMap } = stats;
-  const moduleFiles = modules.reduce(
-    (result, module) => [...result, ...moduleFileMap[module]],
-    []
-  );
+export default (stats, chunks) => {
+  const { entryFiles, vendorFiles } = stats;
+  const chunkFiles = chunks.reduce((result, chunk) => {
+    const chunkGroup = stats.namedChunkGroups[chunk];
+    const assets = chunkGroup.assets.map((asset) =>
+      ensureLeadingSlash(asset.name)
+    );
+    return [...result, ...assets];
+  }, []);
 
-  return [...vendorFiles, ...moduleFiles, ...entryFiles]
+  return [...vendorFiles, ...chunkFiles, ...entryFiles]
     .filter(
       (asset, index, self) =>
         self.indexOf(asset) === index &&

diff --git a/packages/react/package.json b/packages/react/package.json
@@ -23,6 +23,7 @@
   "dependencies": {
     "@babel/core": "^7.9.0",
     "@babel/plugin-proposal-class-properties": "^7.8.3",
+    "@babel/plugin-syntax-jsx": "^7.8.3",
     "@babel/plugin-transform-flow-strip-types": "^7.9.0",
     "@babel/preset-react": "^7.9.4",
     "@pmmmwh/react-refresh-webpack-plugin": "^0.4.3",