[dev-server-legacy] Only transform modules to system js

.changeset/short-months-look.md

@@ -0,0 +1,5 @@
+---
+'@web/test-runner': patch
+---
+
+run user plugins after builtin plugins

.changeset/smart-spiders-smash.md

@@ -0,0 +1,5 @@
+---
+'@web/dev-server-legacy': patch
+---
+
+only transform modules to systemjs

.changeset/sweet-starfishes-laugh.md

@@ -0,0 +1,6 @@
+---
+'@web/dev-server-core': patch
+'@web/dev-server': patch
+---
+
+ensure user plugins are run after builtin plugins

packages/dev-server-core/src/plugins/transformModuleImportsPlugin.ts

@@ -127,7 +127,6 @@ export async function transformImports(
 
     if (dynamicImportIndex === -1) {
       // static import
-
       const importSpecifier = code.substring(start, end);
       const lines = code.slice(0, end).split('\n');
       const line = lines.length;
@@ -258,6 +257,7 @@ export function transformModuleImportsPlugin(plugins: Plugin[], rootDir: string)
             predicates.NOT(predicates.hasAttr('src')),
           ),
         );
+        let transformed = false;
 
         for (const node of inlineModuleNodes) {
           const code = getTextContent(node);
@@ -267,10 +267,15 @@ export function transformModuleImportsPlugin(plugins: Plugin[], rootDir: string)
             rootDir,
             importPlugins,
           );
-          setTextContent(node, resolvedCode);
+          if (code !== resolvedCode) {
+            setTextContent(node, resolvedCode);
+            transformed = true;
+          }
         }
 
-        return { body: serializeHtml(documentAst) };
+        if (transformed) {
+          return { body: serializeHtml(documentAst) };
+        }
       }
     },
   };

packages/dev-server-core/src/server/createPlugins.ts → packages/dev-server-core/src/server/addPlugins.ts

@@ -1,23 +1,23 @@
 import { DevServerCoreConfig } from '../DevServerCoreConfig';
-import { Plugin } from '../Plugin';
 import { transformModuleImportsPlugin } from '../plugins/transformModuleImportsPlugin';
 import { webSocketsPlugin } from '../web-sockets/webSocketsPlugin';
 import { mimeTypesPlugin } from '../plugins/mimeTypesPlugin';
 
-export function createPlugins(config: DevServerCoreConfig) {
-  const plugins: Plugin[] = [];
-
-  if (config.mimeTypes && Object.keys(config.mimeTypes).length > 0) {
-    plugins.push(mimeTypesPlugin(config.mimeTypes));
+export function addPlugins(config: DevServerCoreConfig) {
+  if (!config.plugins) {
+    config.plugins = [];
   }
 
-  if (config.plugins?.some(pl => 'resolveImport' in pl || 'transformImport' in pl)) {
-    plugins.push(transformModuleImportsPlugin(config.plugins, config.rootDir));
+  if (config.mimeTypes && Object.keys(config.mimeTypes).length > 0) {
+    config.plugins.unshift(mimeTypesPlugin(config.mimeTypes));
   }
 
   if (config.injectWebSocket && config.plugins?.some(pl => pl.injectWebSocket)) {
-    plugins.push(webSocketsPlugin());
+    config.plugins.unshift(webSocketsPlugin());
   }
 
-  return plugins;
+  if (config.plugins?.some(pl => 'resolveImport' in pl || 'transformImport' in pl)) {
+    // transform module imports must happen after all other plugins did their regular transforms
+    config.plugins.push(transformModuleImportsPlugin(config.plugins, config.rootDir));
+  }
 }

packages/dev-server-core/src/server/createServer.ts

@@ -9,7 +9,7 @@ import net, { Server, Socket, ListenOptions } from 'net';
 import { DevServerCoreConfig } from '../DevServerCoreConfig';
 import { createMiddleware } from './createMiddleware';
 import { Logger } from '../logger/Logger';
-import { createPlugins } from './createPlugins';
+import { addPlugins } from './addPlugins';
 
 /**
  * A request handler that returns a 301 HTTP Redirect to the same location as the original
@@ -28,22 +28,18 @@ function httpsRedirect(req: IncomingMessage, res: ServerResponse) {
 export function createServer(cfg: DevServerCoreConfig, logger: Logger, fileWatcher: FSWatcher) {
   const app = new Koa();
 
-  const plugins = createPlugins(cfg);
-  if (!cfg.plugins) {
-    cfg.plugins = [];
-  }
-  cfg.plugins.push(...plugins);
+  addPlugins(cfg);
 
   // special case the legacy plugin, if it is given make sure the resolve module imports plugin
   // runs before the legacy plugin because it compiles away module syntax. ideally we have a
   // generic API for this, but we need to design that a bit more first
-  const indexOfLegacy = cfg.plugins.findIndex(p => p.name === 'legacy');
-  let indexOfResolve = cfg.plugins.findIndex(p => p.name === 'resolve-module-imports');
+  const indexOfLegacy = cfg.plugins!.findIndex(p => p.name === 'legacy');
+  let indexOfResolve = cfg.plugins!.findIndex(p => p.name === 'resolve-module-imports');
   if (indexOfLegacy !== -1 && indexOfResolve !== -1) {
-    const legacy = cfg.plugins.splice(indexOfLegacy, 1)[0];
+    const legacy = cfg.plugins!.splice(indexOfLegacy, 1)[0];
     // recompute after splicing
-    indexOfResolve = cfg.plugins.findIndex(p => p.name === 'resolve-module-imports');
-    cfg.plugins.splice(indexOfResolve, 1, cfg.plugins[indexOfResolve], legacy);
+    indexOfResolve = cfg.plugins!.findIndex(p => p.name === 'resolve-module-imports');
+    cfg.plugins!.splice(indexOfResolve, 1, cfg.plugins![indexOfResolve], legacy);
   }
 
   const middleware = createMiddleware(cfg, logger, fileWatcher);

packages/dev-server-import-maps/test/resolving.test.ts

@@ -318,7 +318,7 @@ describe('resolving imports', () => {
     });
 
     const text = await fetchText(`${host}/index.html`);
-    expectIncludes(text, '<html><head></head><body><script src="./app.js"></script></body></html>');
+    expectIncludes(text, '<html><body><script src="./app.js"></script></body></html>');
 
     server.stop();
   });

packages/dev-server-legacy/package.json

@@ -45,6 +45,7 @@
     "@babel/core": "^7.10.5",
     "@babel/plugin-proposal-dynamic-import": "^7.10.4",
     "@babel/plugin-syntax-class-properties": "^7.10.4",
+    "@babel/plugin-syntax-dynamic-import": "^7.8.3",
     "@babel/plugin-syntax-import-meta": "^7.10.4",
     "@babel/plugin-syntax-numeric-separator": "^7.10.4",
     "@babel/plugin-transform-modules-systemjs": "^7.10.5",

packages/dev-server-legacy/src/babelTransform.ts

@@ -1,6 +1,6 @@
 import { transformAsync, TransformOptions } from '@babel/core';
 
-export const config: TransformOptions = {
+export const es5Config: TransformOptions = {
   caller: {
     name: '@web/dev-server-legacy',
     supportsStaticESM: true,
@@ -28,6 +28,14 @@ export const config: TransformOptions = {
     require.resolve('@babel/plugin-syntax-import-meta'),
     require.resolve('@babel/plugin-syntax-class-properties'),
     require.resolve('@babel/plugin-syntax-numeric-separator'),
+    require.resolve('@babel/plugin-syntax-dynamic-import'),
+  ],
+};
+
+export const systemJsConfig: TransformOptions = {
+  ...es5Config,
+  plugins: [
+    ...(es5Config.plugins ?? []),
     require.resolve('@babel/plugin-proposal-dynamic-import'),
     require.resolve('@babel/plugin-transform-modules-systemjs'),
     // systemjs adds template literals, we do systemjs after (potential)
@@ -36,7 +44,7 @@ export const config: TransformOptions = {
   ],
 };
 
-export async function babelTransform(filename: string, source: string) {
+export async function babelTransform(filename: string, source: string, config: TransformOptions) {
   const largeFile = source.length > 100000;
   const result = await transformAsync(source, {
     filename,

packages/dev-server-legacy/src/constants.ts

@@ -0,0 +1 @@
+export const PARAM_TRANSFORM_SYSTEMJS = 'systemjs';

packages/dev-server-legacy/src/injectPolyfillsLoader.ts

@@ -9,6 +9,7 @@ import {
   GeneratedFile,
   File,
 } from 'polyfills-loader';
+import { PARAM_TRANSFORM_SYSTEMJS } from './constants';
 import { findJsScripts } from './findJsScripts';
 
 function findScripts(indexUrl: string, documentAst: DocumentAst) {
@@ -22,13 +23,17 @@ function findScripts(indexUrl: string, documentAst: DocumentAst) {
     let src = getAttribute(scriptNode, 'src');
 
     if (!src) {
-      src = `inline-script-${i}.js?source=${encodeURIComponent(indexUrl)}`;
+      const suffix = type === 'module' ? `&${PARAM_TRANSFORM_SYSTEMJS}=true` : '';
+      src = `inline-script-${i}.js?source=${encodeURIComponent(indexUrl)}${suffix}`;
       inlineScripts.push({
         path: src,
         type,
         content: getTextContent(scriptNode),
       });
       inlineScriptNodes.push(scriptNode);
+    } else if (type === 'module') {
+      const separator = src.includes('?') ? '&' : '?';
+      src = `${src}${separator}${PARAM_TRANSFORM_SYSTEMJS}=true`;
     }
 
     files.push({

packages/dev-server-legacy/src/legacyPlugin.ts

@@ -2,8 +2,9 @@ import { Plugin, Logger, getRequestFilePath, isInlineScriptRequest } from '@web/
 import { GeneratedFile, PolyfillsConfig } from 'polyfills-loader';
 import path from 'path';
 import { isLegacyBrowser } from './isLegacyBrowser';
-import { babelTransform } from './babelTransform';
+import { babelTransform, es5Config, systemJsConfig } from './babelTransform';
 import { injectPolyfillsLoader } from './injectPolyfillsLoader';
+import { PARAM_TRANSFORM_SYSTEMJS } from './constants';
 
 interface inlineScripts {
   lastModified: string;
@@ -103,8 +104,12 @@ export function legacyPlugin(options: LegacyPluginOptions = {}): Plugin {
         if (context.path.includes('/polyfills/')) {
           return;
         }
+        const config =
+          context.URL.searchParams.get(PARAM_TRANSFORM_SYSTEMJS) === 'true'
+            ? systemJsConfig
+            : es5Config;
         const filePath = getRequestFilePath(context, rootDir);
-        const transformed = await babelTransform(filePath, context.body);
+        const transformed = await babelTransform(filePath, context.body, config);
         context.body = transformed;
         return;
       }
@@ -129,6 +134,15 @@ export function legacyPlugin(options: LegacyPluginOptions = {}): Plugin {
         });
       }
     },
+
+    transformImport({ source, context }) {
+      if (!isLegacyBrowser(context, logger)) {
+        return;
+      }
+
+      const prefix = source.includes('?') ? '&' : '?';
+      return `${source}${prefix}${PARAM_TRANSFORM_SYSTEMJS}=true`;
+    },
   };
 }
 

packages/dev-server-legacy/test/transform-html.test.ts

@@ -8,7 +8,8 @@ import { modernUserAgents, legacyUserAgents } from './userAgents';
 const htmlBody = `
 <html>
 <body>
-  <script type="module" src="./foo.js"></scrip>
+  <script type="module" src="./foo.js"></script>
+  <script src="./bar.js"></script>
 </body>
 </html>`;
 
@@ -20,10 +21,11 @@ const inlineScriptHtmlBody = `
 
     }
   </script>
+  <script>console.log("x");</script>
 </body>
 </html>`;
 
-describe('legacyPlugin()', function () {
+describe('legacyPlugin - transform html', function () {
   this.timeout(10000);
 
   it(`does not do any work on a modern browser`, async () => {
@@ -45,6 +47,7 @@ describe('legacyPlugin()', function () {
     const text = await fetchText(`${host}/index.html`, {
       headers: { 'user-agent': modernUserAgents['Chrome 78'] },
     });
+
     expect(text.trim()).to.equal(htmlBody.trim());
     server.stop();
   });
@@ -71,11 +74,34 @@ describe('legacyPlugin()', function () {
     expectIncludes(text, 'function polyfillsLoader() {');
     expectIncludes(text, "loadScript('./polyfills/regenerator-runtime.");
     expectIncludes(text, "loadScript('./polyfills/fetch.");
-    expectIncludes(text, "System.import('./foo.js');");
     expectIncludes(text, "loadScript('./polyfills/systemjs.");
     server.stop();
   });
 
+  it(`injects systemjs param to inline modules`, async () => {
+    const { server, host } = await createTestServer({
+      rootDir: __dirname,
+      plugins: [
+        {
+          name: 'test',
+          serve(context) {
+            if (context.path === '/index.html') {
+              return htmlBody;
+            }
+          },
+        },
+        legacyPlugin(),
+      ],
+    });
+
+    const text = await fetchText(`${host}/index.html`, {
+      headers: { 'user-agent': legacyUserAgents['IE 11'] },
+    });
+    expectIncludes(text, "loadScript('./bar.js');");
+    expectIncludes(text, "System.import('./foo.js?systemjs=true');");
+    server.stop();
+  });
+
   it(`handles inline scripts`, async () => {
     const { server, host } = await createTestServer({
       rootDir: __dirname,
@@ -95,7 +121,11 @@ describe('legacyPlugin()', function () {
     const text = await fetchText(`${host}/index.html`, {
       headers: { 'user-agent': legacyUserAgents['IE 11'] },
     });
-    expectIncludes(text, "System.import('./inline-script-0.js?source=%2Findex.html');");
+    expectIncludes(text, "loadScript('./inline-script-0.js?source=%2Findex.html');");
+    expectIncludes(
+      text,
+      "System.import('./inline-script-1.js?source=%2Findex.html&systemjs=true');",
+    );
     server.stop();
   });
 
@@ -118,12 +148,11 @@ describe('legacyPlugin()', function () {
     await fetchText(`${host}/index.html`, {
       headers: { 'user-agent': legacyUserAgents['IE 11'] },
     });
-    const text = await fetchText(`${host}/inline-script-0.js?source=%2Findex.html`, {
+    const text = await fetchText(`${host}/inline-script-1.js?source=%2Findex.html`, {
       headers: { 'user-agent': legacyUserAgents['IE 11'] },
     });
-    expectIncludes(text, 'System.register');
-    expectIncludes(text, 'var InlineClass;');
-    expectIncludes(text, 'function _classCallCheck(instance');
+    expectIncludes(text, 'var InlineClass =');
+    expectIncludes(text, '_classCallCheck(this, InlineClass);');
     server.stop();
   });