fix(rollup-plugin-html): handle race conditions with multi build

packages/rollup-plugin-html/README.md

@@ -353,8 +353,6 @@ To do this, create one parent `@open-wc/rollup-plugin-html` instance and use `ad
 
 Each output defines a unique name, this can be used to retreive the correct bundle from `bundles` argument when creating the HTML template.
 
-The HTML file will be output into the directory of the last build. If your builds will be output into separate directories, you need to make sure the main directory in the last.
-
 ```js
 import html from '@open-wc/rollup-plugin-html';
 
@@ -382,18 +380,55 @@ const htmlPlugin = html({
 export default {
   input: './app.js',
   output: [
+    {
+      format: 'es',
+      dir: 'dist',
+      plugins: [htmlPlugin.addOutput('modern')],
+    },
     {
       format: 'system',
-      dir: 'dist/legacy',
+      dir: 'dist',
       plugins: [htmlPlugin.addOutput('legacy')],
     },
-    // Note: the modern build should always be last, as the HTML file will be output into
-    // this directory
+  ],
+  plugins: [htmlPlugin],
+};
+```
+
+</details>
+
+If your outputs use different outputs directories, you need to set the `outputBundleName` option to specify which build to use to output the HTML file.
+
+<details>
+<summary>View example</summary>
+
+```js
+import html from '@open-wc/rollup-plugin-html';
+
+const htmlPlugin = html({
+  name: 'index.html',
+  inject: false,
+  outputBundleName: 'modern',
+  template({ bundles }) {
+    return `
+      ...
+    `;
+  },
+});
+
+export default {
+  input: './app.js',
+  output: [
     {
       format: 'es',
       dir: 'dist',
       plugins: [htmlPlugin.addOutput('modern')],
     },
+    {
+      format: 'system',
+      dir: 'dist',
+      plugins: [htmlPlugin.addOutput('legacy')],
+    },
   ],
   plugins: [htmlPlugin],
 };
@@ -423,6 +458,12 @@ Type: `string`
 
 Same as `inputPath`, but provides the HTML as a string directly.
 
+### outputBundleName
+
+Type: `string`
+
+When using multiple build outputs, this is the name of the build that will be used to emit the generated HTML file.
+
 ### dir
 
 Type: `string`
@@ -480,7 +521,7 @@ export interface PluginOptions {
   name?: string;
   inputPath?: string;
   inputHtml?: string;
-  dir?: string;
+  outputBundleName?: string;
   publicPath?: string;
   inject?: boolean;
   minify?: boolean | object | MinifyFunction;

packages/rollup-plugin-html/rollup-plugin-html.js

@@ -6,6 +6,7 @@
 /** @typedef {import('rollup').OutputBundle} OutputBundle */
 /** @typedef {import('rollup').OutputChunk} OutputChunk */
 /** @typedef {import('rollup').EmittedFile} EmittedFile */
+/** @typedef {import('rollup').EmitFile} EmitFile */
 /** @typedef {import('./src/types').PluginOptions} PluginOptions */
 /** @typedef {import('./src/types').InputHtmlData} InputHtmlData */
 /** @typedef {import('./src/types').GeneratedBundle} GeneratedBundle */
@@ -18,12 +19,7 @@ const { getInputHtmlData } = require('./src/getInputHtmlData');
 const { getEntrypointBundles } = require('./src/getEntrypointBundles');
 const { getOutputHtml } = require('./src/getOutputHtml');
 const { extractModules } = require('./src/extractModules');
-const {
-  createError,
-  getMainOutputDir,
-  addRollupInput,
-  shouldReadInputFromRollup,
-} = require('./src/utils');
+const { createError, addRollupInput, shouldReadInputFromRollup } = require('./src/utils');
 
 const watchMode = process.env.ROLLUP_WATCH === 'true';
 const defaultFileName = 'index.html';
@@ -39,10 +35,6 @@ function rollupPluginHtml(pluginOptions) {
     ...(pluginOptions || {}),
   };
 
-  /** @type {string[]} */
-  const multiOutputNames = [];
-  let multiOutput = false;
-  let outputCount = 0;
   /** @type {string} */
   let inputHtml;
   /** @type {string[]} */
@@ -56,16 +48,24 @@ function rollupPluginHtml(pluginOptions) {
   /** @type {TransformFunction[]} */
   let externalTransformFns = [];
 
+  // variables for multi build
+  /** @type {string[]} */
+  const multiOutputNames = [];
+  // function emit asset, used when the outputName option is
+  // used to explicitly configure which output build to emit
+  // assets from
+  /** @type {Function} */
+  let deferredEmitHtmlFile;
+
   /**
+   * @param {string} mainOutputDir
    * @returns {Promise<EmittedFile>}
    */
-  async function createHtmlAsset() {
+  async function createHtmlAsset(mainOutputDir) {
     if (generatedBundles.length === 0) {
       throw createError('Cannot output HTML when no bundles have been generated');
     }
 
-    const mainOutputDir = getMainOutputDir(pluginOptions, generatedBundles);
-
     const entrypointBundles = getEntrypointBundles({
       pluginOptions,
       generatedBundles,
@@ -167,9 +167,12 @@ function rollupPluginHtml(pluginOptions) {
      * @param {OutputBundle} bundle
      */
     async generateBundle(options, bundle) {
-      if (multiOutput) return;
+      if (multiOutputNames.length !== 0) return;
+      if (!options.dir) {
+        throw createError('Output must have a dir option set.');
+      }
       generatedBundles.push({ name: 'default', options, bundle });
-      this.emitFile(await createHtmlAsset());
+      this.emitFile(await createHtmlAsset(options.dir));
     },
 
     getHtmlFileName() {
@@ -193,13 +196,11 @@ function rollupPluginHtml(pluginOptions) {
       if (!name || multiOutputNames.includes(name)) {
         throw createError('Each output must have a unique name');
       }
+
       multiOutputNames.push(name);
 
-      multiOutput = true;
-      outputCount += 1;
-
       return {
-        name: `rollup-plugin-html-multi-output-${outputCount}`,
+        name: `rollup-plugin-html-multi-output-${multiOutputNames.length}`,
 
         /**
          * Stores output bundle, and emits output HTML file if all builds
@@ -208,10 +209,63 @@ function rollupPluginHtml(pluginOptions) {
          * @param {OutputBundle} bundle
          */
         async generateBundle(options, bundle) {
-          generatedBundles.push({ name, options, bundle });
-          if (generatedBundles.length === outputCount) {
-            this.emitFile(await createHtmlAsset());
-          }
+          return new Promise(resolve => {
+            if (!options.dir) {
+              throw createError(`Output ${name} must have a dir option set.`);
+            }
+
+            generatedBundles.push({ name, options, bundle });
+
+            if (pluginOptions.outputBundleName) {
+              if (!multiOutputNames.includes(pluginOptions.outputBundleName)) {
+                throw createError(
+                  `outputName is set to ${pluginOptions.outputBundleName} but there was no ` +
+                    "output added with this name. Used .addOutput('name') to add it.",
+                );
+              }
+
+              if (pluginOptions.outputBundleName === name) {
+                // we need to emit the asset from this output's asset tree, but not all build outputs have
+                // finished building yet. create a function to be called later to emit the final output
+                // when all builds are finished
+                const { dir } = options;
+                deferredEmitHtmlFile = () =>
+                  createHtmlAsset(dir).then(asset => {
+                    this.emitFile(asset);
+                    resolve();
+                  });
+              }
+            }
+
+            if (generatedBundles.length === multiOutputNames.length) {
+              // this is the last build, emit the HTML file
+              if (deferredEmitHtmlFile) {
+                // emit to another build output's asset tree
+                deferredEmitHtmlFile().then(() => {
+                  resolve();
+                });
+                return;
+              }
+
+              const outputDirs = new Set(generatedBundles.map(b => b.options.dir));
+              if (outputDirs.size !== 1) {
+                throw createError(
+                  `Multiple rollup build outputs have a different output directory set.` +
+                    ' Set the outputName property to indicate which build should be used to emit the generated HTML file.',
+                );
+              }
+
+              // emit asset from this output
+              createHtmlAsset(options.dir).then(asset => {
+                resolve();
+                this.emitFile(asset);
+              });
+              return;
+            }
+
+            // no work to be done for this build output
+            resolve();
+          });
         },
       };
     },

packages/rollup-plugin-html/src/types.d.ts

@@ -4,7 +4,7 @@ export interface PluginOptions {
   name?: string;
   inputPath?: string;
   inputHtml?: string;
-  dir?: string;
+  outputBundleName?: string;
   publicPath?: string;
   inject?: boolean;
   minify?: boolean | object | MinifyFunction;

packages/rollup-plugin-html/src/utils.js

@@ -25,26 +25,6 @@ function fromEntries(entries) {
   return obj;
 }
 
-/**
- * @param {PluginOptions} pluginOptions
- * @param {GeneratedBundle[]} generatedBundles
- * @returns {string}
- */
-function getMainOutputDir(pluginOptions, generatedBundles) {
-  const mainOutputDir =
-    // user defined output dir
-    pluginOptions.dir ||
-    // if no used defined output dir, we find the "lowest" output dir, ex. if there are
-    // "dist/legacy" and "dist", we take "dist"
-    generatedBundles.map(b => b.options.dir).sort((a, b) => (a && b ? a.length - b.length : 0))[0];
-
-  if (typeof mainOutputDir !== 'string')
-    throw createError(
-      "Rollup must be configured to output in a directory: html({ outputDir: 'dist' })",
-    );
-  return mainOutputDir;
-}
-
 /**
  * @param {InputOptions} inputOptions
  * @param {string[]} inputModuleIds
@@ -93,7 +73,6 @@ function shouldReadInputFromRollup(rollupInputOptions, pluginOptions) {
 
 module.exports = {
   createError,
-  getMainOutputDir,
   fromEntries,
   addRollupInput,
   shouldReadInputFromRollup,

packages/rollup-plugin-html/test/rollup-plugin-html.test.js

@@ -270,7 +270,7 @@ describe('rollup-plugin-html', () => {
     const build = await rollup.rollup(config);
     const bundleA = build.generate({
       format: 'system',
-      dir: 'dist/legacy',
+      dir: 'dist',
       plugins: [plugin.addOutput('legacy')],
     });
     const bundleB = build.generate({
@@ -293,7 +293,53 @@ describe('rollup-plugin-html', () => {
     expect(getAsset(outputA, 'index.html')).to.not.exist;
     expect(getAsset(outputB, 'index.html').source).to.equal(
       '<html><head></head><body><h1>Hello world</h1>' +
-        '<script>System.import("/static/legacy/entrypoint-a.js");</script>' +
+        '<script>System.import("/static/entrypoint-a.js");</script>' +
+        '<script type="module" src="/static/entrypoint-a.js"></script></body></html>',
+    );
+  });
+
+  it('can build with multiple build outputs, specifying a specific output directory', async () => {
+    const plugin = htmlPlugin({
+      name: 'index.html',
+      outputBundleName: 'modern',
+      inputHtml:
+        '<h1>Hello world</h1>' +
+        '<script type="module" src="./test/fixtures/rollup-plugin-html/entrypoint-a.js"></script>',
+      publicPath: '/static/',
+      minify: false,
+    });
+
+    const config = {
+      input: './test/fixtures/rollup-plugin-html/entrypoint-a.js',
+      plugins: [plugin],
+    };
+
+    const build = await rollup.rollup(config);
+    const bundleA = build.generate({
+      format: 'system',
+      dir: 'dist-1',
+      plugins: [plugin.addOutput('legacy')],
+    });
+    const bundleB = build.generate({
+      format: 'es',
+      dir: 'dist-2',
+      plugins: [plugin.addOutput('modern')],
+    });
+
+    const [{ output: outputA }, { output: outputB }] = await Promise.all([bundleA, bundleB]);
+
+    expect(outputA.length).to.equal(1);
+    expect(outputB.length).to.equal(2);
+    const { code: entrypointA1 } = getChunk(outputA, 'entrypoint-a.js');
+    const { code: entrypointA2 } = getChunk(outputB, 'entrypoint-a.js');
+    expect(entrypointA1).to.include("console.log('entrypoint-a.js');");
+    expect(entrypointA1).to.include("console.log('module-a.js');");
+    expect(entrypointA2).to.include("console.log('entrypoint-a.js');");
+    expect(entrypointA2).to.include("console.log('module-a.js');");
+    expect(getAsset(outputA, 'index.html')).to.not.exist;
+    expect(getAsset(outputB, 'index.html').source).to.equal(
+      '<html><head></head><body><h1>Hello world</h1>' +
+        '<script>System.import("/dist-1/entrypoint-a.js");</script>' +
         '<script type="module" src="/static/entrypoint-a.js"></script></body></html>',
     );
   });