[bundler] Fixed the problem where we totally failed at inlining external module script tags.

packages/build/src/bundle.ts

@@ -152,18 +152,20 @@ export class BuildBundler extends AsyncTransformStream<File, File> {
    */
   private _unmapBundledFiles(manifest: BundleManifest) {
     for (const {
+           files,
            inlinedHtmlImports,
            inlinedScripts,
            inlinedStyles,
-           bundledExports,
          } of manifest.bundles.values()) {
       for (const url
-               of [...inlinedHtmlImports,
+               of [...files,
+                   ...inlinedHtmlImports,
                    ...inlinedScripts,
-                   ...inlinedStyles,
-                   ...bundledExports.keys()]) {
-        this.files.delete(url);
+                   ...inlinedStyles]) {
+        // Don't unmap the bundle file url itself.
+        if (!manifest.bundles.has(url)) {
+          this.files.delete(url);
+        }
       }
     }
   }
-}

packages/bundler/src/html-bundler.ts

@@ -17,7 +17,7 @@ import * as dom5 from 'dom5';
 import {ASTNode, parseFragment, serialize, treeAdapters} from 'parse5';
 import {Document, FileRelativeUrl, ParsedHtmlDocument, ResolvedUrl} from 'polymer-analyzer';
 
-import {assertIsHtmlDocument, getAnalysisDocument} from './analyzer-utils';
+import {assertIsHtmlDocument, assertIsJsDocument, getAnalysisDocument} from './analyzer-utils';
 import {AssignedBundle, BundleManifest} from './bundle-manifest';
 import {Bundler} from './bundler';
 import constants from './constants';
@@ -72,13 +72,13 @@ export class HtmlBundler {
     // imports, since we may now have appended some that were not initially
     // present.
     this.document = await this._reanalyze(serialize(ast));
-
     await this._inlineHtmlImports(ast);
 
-    await this._updateExternalModuleScripts(ast);
+    await this._updateExternalModuleScriptTags(ast);
     if (this.bundler.enableScriptInlining) {
       await this._inlineNonModuleScripts(ast);
-      await this._inlineModuleScripts(ast);
+      await this._rewriteExternalModuleScriptTagsAsImports(ast);
+      await this._rollupInlineModuleScripts(ast);
     }
     if (this.bundler.enableCssInlining) {
       await this._inlineStylesheetLinks(ast);
@@ -426,19 +426,24 @@ export class HtmlBundler {
    * Update the `src` attribute of external `type=module` script tags to point
    * at new bundle locations.
    */
-  public async _updateExternalModuleScripts(ast: ASTNode) {
+  public async _updateExternalModuleScriptTags(ast: ASTNode) {
     const scripts = dom5.queryAll(ast, matchers.externalModuleScript);
     for (const script of scripts) {
       const oldSrc = dom5.getAttribute(script, 'src');
       const oldFileUrl = this.bundler.analyzer.urlResolver.resolve(
-          this.assignedBundle.url, oldSrc as FileRelativeUrl);
+          this.document.parsedDocument.baseUrl, oldSrc as FileRelativeUrl);
       if (oldFileUrl === undefined) {
         continue;
       }
       const bundle = this.manifest.getBundleForFile(oldFileUrl);
       if (bundle === undefined) {
         continue;
       }
+      // Do not rewrite the src if the current bundle is going to be the new
+      // home of the code.
+      if (bundle.url === this.assignedBundle.url) {
+        continue;
+      }
       const newFileUrl = bundle.url;
       const newSrc = this.bundler.analyzer.urlResolver.relative(
           this.assignedBundle.url, newFileUrl);
@@ -450,7 +455,7 @@ export class HtmlBundler {
    * Inlines the contents of external module scripts and rolls-up imported
    * modules into inline scripts.
    */
-  private async _inlineModuleScripts(ast: ASTNode) {
+  private async _rollupInlineModuleScripts(ast: ASTNode) {
     this.document = await this._reanalyze(serialize(ast));
     rewriteObject(ast, this.document.parsedDocument.ast);
     dom5.removeFakeRootElements(ast);
@@ -481,14 +486,42 @@ export class HtmlBundler {
     }
   }
 
+  /**
+   * Replace all external module script tags:
+   * `<script type="module" src="..."></script>`
+   * with inline script tags containing import:
+   * `<script type="module">import '...';</script>`
+   * And these will be subsequently rolled up by call to
+   * `this._rollupInlineModuleScripts()`.
+   */
+  private async _rewriteExternalModuleScriptTagsAsImports(ast: ASTNode) {
+    for (const scriptTag of dom5.queryAll(ast, matchers.externalModuleScript)) {
+      const scriptHref = dom5.getAttribute(scriptTag, 'src')!;
+      const resolvedImportUrl = this.bundler.analyzer.urlResolver.resolve(
+          this.document.parsedDocument.baseUrl, scriptHref as FileRelativeUrl);
+      if (resolvedImportUrl === undefined) {
+        return;
+      }
+      // We won't inline a module script if its not supposed to be in this
+      // bundle.
+      if (!this.assignedBundle.bundle.files.has(resolvedImportUrl)) {
+        return;
+      }
+      const scriptContent = `import ${JSON.stringify(scriptHref)};`;
+      dom5.removeAttribute(scriptTag, 'src');
+      dom5.setTextContent(scriptTag, encodeString(scriptContent, true));
+    }
+  }
+
+
   /**
    * Inlines the contents of the document returned by the script tag's src URL
    * into the script tag content and removes the src attribute.
    */
   private async _inlineNonModuleScript(scriptTag: ASTNode) {
     const scriptHref = dom5.getAttribute(scriptTag, 'src')!;
     const resolvedImportUrl = this.bundler.analyzer.urlResolver.resolve(
-        this.assignedBundle.url, scriptHref as FileRelativeUrl);
+        this.document.parsedDocument.baseUrl, scriptHref as FileRelativeUrl);
     if (resolvedImportUrl === undefined) {
       return;
     }

packages/bundler/src/test/html-bundler_test.ts

@@ -31,6 +31,30 @@ const stripSpace = (html: string): string =>
 
 suite('HtmlBundler', () => {
 
+  test('external script tag inlines an es6 module', async () => {
+    const root = 'test/html/inline-es6-modules';
+    const analyzer = new Analyzer({
+      urlResolver: new FsUrlResolver(root),
+      urlLoader: new FsUrlLoader(root),
+      moduleResolution: 'node',
+    });
+    const bundler = new Bundler({analyzer});
+    const externalScriptToNodeModuleUrl =
+        analyzer.resolveUrl('external-script-to-node-module.html')!;
+    const {documents} = await bundler.bundle(
+        await bundler.generateManifest([externalScriptToNodeModuleUrl]));
+    const externalScriptToNodeModuleDoc =
+        documents.getHtmlDoc(externalScriptToNodeModuleUrl)!;
+    assert.deepEqual(externalScriptToNodeModuleDoc.content, heredoc`
+      <script type="module">
+      const feature = {
+        cool: 'thing'
+      };
+      console.log('imported some-package/main.js');
+      </script>
+    `);
+  });
+
   test('inline es6 modules with node module-resolution', async () => {
     const root = 'test/html/inline-es6-modules';
     const analyzer = new Analyzer({
@@ -50,6 +74,7 @@ suite('HtmlBundler', () => {
       const feature = {
         cool: 'thing'
       };
+      console.log('imported some-package/main.js');
       console.log(feature);
       </script>
     `);

packages/bundler/test/html/inline-es6-modules/external-script-to-node-module.html

@@ -0,0 +1 @@
+<script type="module" src="./node_modules/some-package/lib/main.js"></script>

packages/cli/src/test/integration/fixtures/build-modules/expected/bundled-inlined-es/index.html

@@ -0,0 +1,8 @@
+<html><head>
+    <script type="module">const dep1 = 'dep1';
+const mod2 = 'mod2';
+const mod1 = 'mod1';</script>
+  </head>
+  <body>
+
+</body></html>

packages/cli/src/test/integration/fixtures/build-modules/expected/polymer.json

@@ -34,6 +34,10 @@
       "js": {
         "transformModulesToAmd": true
       }
+    },
+    {
+      "name": "bundled-inlined-es",
+      "bundle": true
     }
   ],
   "npm": true,

packages/cli/src/test/integration/fixtures/build-modules/source/polymer.json

@@ -27,6 +27,10 @@
       "js": {
         "transformModulesToAmd": true
       }
+    },
+    {
+      "name": "bundled-inlined-es",
+      "bundle": true
     }
   ]
 }