Esbuild improvements

.changeset/grumpy-points-report.md

@@ -0,0 +1,6 @@
+---
+'modular-scripts': minor
+---
+
+Output file structure equivalent to webpack for esbuild files
+`/static/(js|css)/*`.

.changeset/nervous-pianos-live.md

@@ -0,0 +1,6 @@
+---
+'modular-scripts': patch
+---
+
+Reduced number of plugin passes required to generate SVGs using SVGR in esbuild
+output.

.changeset/orange-bulldogs-beg.md

@@ -0,0 +1,6 @@
+---
+'modular-scripts': patch
+---
+
+Write files in memory when running an esbuild server to fix issues with file
+paths outside of the modular root.

.changeset/shiny-glasses-glow.md

@@ -0,0 +1,6 @@
+---
+'modular-scripts': minor
+---
+
+Improve source map asset paths to be relative to the modular root instead of
+application root.

package.json

@@ -59,6 +59,7 @@
     "@types/global-modules": "^2.0.0",
     "@types/is-ci": "3.0.0",
     "@types/jest": "26.0.24",
+    "@types/mime": "^2.0.3",
     "@types/node": "*",
     "@types/npm-packlist": "1.1.2",
     "@types/parse5": "^6.0.3",
@@ -83,7 +84,7 @@
     "micromatch": "4.0.4",
     "patch-package": "^6.4.7",
     "pptr-testing-library": "0.6.5",
-    "prettier": "2.5.0",
+    "prettier": "2.4.0",
     "puppeteer": "12.0.1",
     "react": "17.0.2",
     "react-dom": "17.0.2",

packages/modular-scripts/package.json

@@ -78,6 +78,7 @@
     "jest-watch-typeahead": "0.6.5",
     "loader-utils": "2.0.0",
     "micromatch": "4.0.4",
+    "mime": "^3.0.0",
     "mini-css-extract-plugin": "0.11.3",
     "npm-packlist": "3.0.0",
     "open": "8.3.0",
@@ -121,9 +122,9 @@
     "ws": "7.4.6"
   },
   "peerDependencies": {
-    "typescript": "^4.3.5",
     "react": "^16 || ^17",
-    "react-dom": "^16 || ^17"
+    "react-dom": "^16 || ^17",
+    "typescript": "^4.3.5"
   },
   "files": [
     "dist-cjs",

packages/modular-scripts/src/__tests__/app.esbuild.test.ts

@@ -76,23 +76,24 @@ describe('when working with an app', () => {
   });
 
   it('can build an app', () => {
-    expect(
-      tree(path.join(modularRoot, 'dist', 'sample-esbuild-app'), {
-        hashIgnores: ['index.js.map', 'index.css.map', 'package.json'],
-      }),
-    ).toMatchInlineSnapshot(`
+    expect(tree(path.join(modularRoot, 'dist', 'sample-esbuild-app')))
+      .toMatchInlineSnapshot(`
       "sample-esbuild-app
       ├─ favicon.ico #6pu3rg
-      ├─ index.css #90bbzw
-      ├─ index.css.map
-      ├─ index.html #ojdrji
-      ├─ index.js #w0zp6v
-      ├─ index.js.map
-      ├─ logo-PGX3QVVN.svg #1okqmlj
+      ├─ index.html #7q1ane
       ├─ logo192.png #1nez7vk
       ├─ logo512.png #1hwqvcc
       ├─ manifest.json #19gah8o
-      └─ robots.txt #1sjb8b3"
+      ├─ robots.txt #1sjb8b3
+      └─ static
+         ├─ css
+         │  ├─ index-OPRZV2UT.css #1ldttcq
+         │  └─ index-OPRZV2UT.css.map #za6yi0
+         ├─ js
+         │  ├─ index-SYRAECOS.js #nswngj
+         │  └─ index-SYRAECOS.js.map #1yt4p38
+         └─ media
+            └─ logo-PGX3QVVN.svg #1okqmlj"
     `);
   });
 
@@ -116,7 +117,14 @@ describe('when working with an app', () => {
       prettier.format(
         String(
           await fs.readFile(
-            path.join(modularRoot, 'dist', 'sample-esbuild-app', 'index.js'),
+            path.join(
+              modularRoot,
+              'dist',
+              'sample-esbuild-app',
+              'static',
+              'js',
+              'index-SYRAECOS.js',
+            ),
           ),
         ),
         {
@@ -141,10 +149,12 @@ describe('when working with an app', () => {
   };
 
   it('can generate a index.js.map', () => {
-    expect(readSourceMap('index.js.map')).toMatchSnapshot();
+    expect(readSourceMap('static/js/index-SYRAECOS.js.map')).toMatchSnapshot();
   });
 
   it('can generate a index.css.map', () => {
-    expect(readSourceMap('index.css.map')).toMatchSnapshot();
+    expect(
+      readSourceMap('static/css/index-OPRZV2UT.css.map'),
+    ).toMatchSnapshot();
   });
 });

packages/modular-scripts/src/__tests__/esbuild-scripts/__snapshots__/svgr.test.tsx.snap

@@ -46,7 +46,7 @@ ReactDOM.render(/* @__PURE__ */ React.createElement(\\"div\\", {
 `;
 
 exports[`WHEN running esbuild with the svgrPlugin WHEN there's a url import SHOULD ouput the correct index.js 1`] = `
-"// svgurl:/packages/modular-scripts/src/__tests__/esbuild-scripts/__fixtures__/svgr-url/logo.svg
+"// svgurl:packages/modular-scripts/src/__tests__/esbuild-scripts/__fixtures__/svgr-url/logo.svg
 var logo_default = \\"./logo-5JCTDEME.svg\\";
 
 // svgr:packages/modular-scripts/src/__tests__/esbuild-scripts/__fixtures__/svgr-url/logo.svg

packages/modular-scripts/src/build/esbuildFileSizeReporter.ts

@@ -5,6 +5,7 @@ import * as path from 'path';
 import recursive from 'recursive-readdir';
 
 import type { Paths } from '../utils/createPaths';
+import getModularRoot from '../utils/getModularRoot';
 import { Asset, canReadAsset } from './fileSizeReporter';
 
 function removeFileNameHash(fileName: string): string {
@@ -19,6 +20,8 @@ function removeFileNameHash(fileName: string): string {
 export function esbuildMeasureFileSizesBeforeBuild(
   buildFolder: string,
 ): Promise<Record<string, number>> {
+  const modularRoot = getModularRoot();
+
   return new Promise<Record<string, number>>((resolve) => {
     recursive(buildFolder, (err: Error, fileNames: string[]) => {
       if (err) {
@@ -28,13 +31,11 @@ export function esbuildMeasureFileSizesBeforeBuild(
           fileNames
             .filter(canReadAsset)
             .reduce<Record<string, number>>((memo, absoluteFilePath) => {
-              const filePath = path.relative(buildFolder, absoluteFilePath);
+              const filePath = path.relative(modularRoot, absoluteFilePath);
 
               const contents = fs.readFileSync(absoluteFilePath);
-              const folder = path.join(
-                path.basename(buildFolder),
-                path.dirname(filePath),
-              );
+
+              const folder = path.dirname(filePath);
               const name = path.basename(filePath);
 
               const key = `${folder}/${removeFileNameHash(name)}`;
@@ -53,20 +54,16 @@ export function createEsbuildAssets(
   paths: Paths,
   stats: esbuild.Metafile,
 ): Asset[] {
-  const readableAssets = Object.keys(stats.outputs)
-    .map((name) => {
-      return path.relative(paths.appBuild, path.join(paths.appPath, name));
-    })
-    .filter(canReadAsset);
+  const modularRoot = getModularRoot();
+
+  const readableAssets = Object.keys(stats.outputs).filter(canReadAsset);
 
   return readableAssets
     .map<Asset>((filePath) => {
-      const fileContents = fs.readFileSync(path.join(paths.appBuild, filePath));
+      const fileContents = fs.readFileSync(path.join(modularRoot, filePath));
       const size = gzipSize(fileContents);
-      const folder = path.join(
-        path.basename(paths.appBuild),
-        path.dirname(filePath),
-      );
+      const folder = path.dirname(filePath);
+
       const name = path.basename(filePath);
       return {
         folder,

packages/modular-scripts/src/build/index.ts

@@ -1,6 +1,5 @@
 import { paramCase as toParamCase } from 'change-case';
 import chalk from 'chalk';
-import type * as esbuild from 'esbuild';
 import * as fs from 'fs-extra';
 import * as path from 'path';
 
@@ -83,7 +82,7 @@ async function buildApp(target: string) {
     );
     const result = await buildEsbuildApp(target, paths);
 
-    assets = createEsbuildAssets(paths, result.metafile as esbuild.Metafile);
+    assets = createEsbuildAssets(paths, result);
   } else {
     // create-react-app doesn't support plain module outputs yet,
     // so --preserve-modules has no effect here
@@ -93,7 +92,6 @@ async function buildApp(target: string) {
     );
 
     const browserTarget = createEsbuildBrowserslistTarget(targetDirectory);
-    logger.debug(`Using target: ${browserTarget.join(', ')}`);
 
     // TODO: this shouldn't be sync
     await execAsync('node', [buildScript], {

packages/modular-scripts/src/esbuild-scripts/api.ts

@@ -1,11 +1,38 @@
+import type * as esbuild from 'esbuild';
 import * as fs from 'fs-extra';
 import * as parse5 from 'parse5';
 import escapeStringRegexp from 'escape-string-regexp';
 
 import type { Paths } from '../utils/createPaths';
+import getModularRoot from '../utils/getModularRoot';
+import * as path from 'path';
+
+type FileType = '.css' | '.js';
+
+function getEntryPoint(
+  paths: Paths,
+  metafile: esbuild.Metafile,
+  type: FileType,
+): string | undefined {
+  const result = Object.entries(metafile.outputs).find(([key, output]) => {
+    return output.entryPoint && path.extname(key) === type;
+  });
+
+  const outputFileName = result?.[0];
+
+  if (outputFileName) {
+    return path.relative(
+      paths.appBuild,
+      path.join(getModularRoot(), outputFileName),
+    );
+  } else {
+    return undefined;
+  }
+}
 
 export async function createIndex(
   paths: Paths,
+  metafile: esbuild.Metafile,
   replacements: Record<string, string>,
   includeRuntime: boolean,
 ): Promise<string> {
@@ -17,19 +44,28 @@ export async function createIndex(
   const head = html.childNodes.find(
     (node) => node.nodeName === 'head',
   ) as parse5.Element;
-  head.childNodes.push(
-    ...parse5.parseFragment(
-      `<link rel="stylesheet" href="%PUBLIC_URL%/index.css"></script>`,
-    ).childNodes,
-  );
+
+  const cssEntryPoint = getEntryPoint(paths, metafile, '.css');
+  const jsEntryPoint = getEntryPoint(paths, metafile, '.js');
+
+  if (cssEntryPoint) {
+    head.childNodes.push(
+      ...parse5.parseFragment(
+        `<link rel="stylesheet" href="%PUBLIC_URL%/${cssEntryPoint}"></script>`,
+      ).childNodes,
+    );
+  }
   const body = html.childNodes.find(
     (node) => node.nodeName === 'body',
   ) as parse5.Element;
-  body.childNodes.push(
-    ...parse5.parseFragment(
-      `<script type="module" src="%PUBLIC_URL%/index.js"></script>`,
-    ).childNodes,
-  );
+
+  if (jsEntryPoint) {
+    body.childNodes.push(
+      ...parse5.parseFragment(
+        `<script type="module" src="%PUBLIC_URL%/${jsEntryPoint}"></script>`,
+      ).childNodes,
+    );
+  }
 
   if (includeRuntime) {
     body.childNodes.push(

packages/modular-scripts/src/esbuild-scripts/build/index.ts

@@ -11,11 +11,50 @@ import { formatError } from '../utils/formatError';
 
 import { createIndex } from '../api';
 import createEsbuildConfig from '../config/createEsbuildConfig';
+import getModularRoot from '../../utils/getModularRoot';
+import sanitizeMetafile from '../utils/sanitizeMetafile';
 
 export default async function build(target: string, paths: Paths) {
+  const modularRoot = getModularRoot();
+
   const env = getClientEnvironment(paths.publicUrlOrPath.slice(0, -1));
 
-  const html = await createIndex(paths, env.raw, false);
+  let result: esbuild.Metafile;
+  try {
+    const buildResult = await esbuild.build(
+      createEsbuildConfig(paths, {
+        entryNames: 'static/js/[name]-[hash]',
+        chunkNames: 'static/js/[name]-[hash]',
+        assetNames: 'static/media/[name]-[hash]',
+      }),
+    );
+
+    result = sanitizeMetafile(paths, buildResult.metafile as esbuild.Metafile);
+  } catch (e) {
+    const result = e as esbuild.BuildFailure;
+    logger.log(chalk.red('Failed to compile.\n'));
+    const logs = result.errors.map(async (m) => {
+      logger.log(await formatError(m, paths.appPath));
+    });
+
+    await Promise.all(logs);
+
+    throw new Error(`Failed to build ${target}`);
+  }
+
+  // move CSS files to their real location on disk...
+  for (const outputFileName of Object.keys(result.outputs)) {
+    if (['.css', '.css.map'].some((ext) => outputFileName.endsWith(ext))) {
+      const cssFileToMove = outputFileName.replace('/css/', '/js/');
+      logger.debug(`Moving css ${cssFileToMove} => ${outputFileName}`);
+      await fs.move(
+        path.join(modularRoot, cssFileToMove),
+        path.join(modularRoot, outputFileName),
+      );
+    }
+  }
+
+  const html = await createIndex(paths, result, env.raw, false);
   await fs.writeFile(
     path.join(paths.appBuild, 'index.html'),
     minimize.minify(html, {
@@ -32,17 +71,5 @@ export default async function build(target: string, paths: Paths) {
     }),
   );
 
-  try {
-    return esbuild.build(createEsbuildConfig(paths));
-  } catch (e) {
-    const result = e as esbuild.BuildFailure;
-    logger.log(chalk.red('Failed to compile.\n'));
-    const logs = result.errors.map(async (m) => {
-      logger.log(await formatError(m, paths.appPath));
-    });
-
-    await Promise.all(logs);
-
-    throw new Error(`Failed to build ${target}`);
-  }
+  return result;
 }

packages/modular-scripts/src/esbuild-scripts/config/createEsbuildConfig.ts

@@ -61,16 +61,15 @@ export default function createEsbuildConfig(
     },
     logLevel: 'silent',
     target,
-    absWorkingDir: paths.appPath,
     format: 'esm',
     color: !isCi,
     define,
     metafile: true,
     tsconfig: paths.appTsConfig,
     minify: true,
-    outbase: 'src',
+    outbase: paths.modularRoot,
+    absWorkingDir: paths.modularRoot,
     outdir: paths.appBuild,
-    sourceRoot: paths.modularRoot,
     publicPath: paths.publicUrlOrPath,
     nodePaths: (process.env.NODE_PATH || '').split(path.delimiter),
     ...partialConfig,