Add tests for external rollup option support (#90)

* Upgrade to latest acorn

* All tests passing

Author: kristoferbaxter

package-lock.json

@@ -30,7 +30,7 @@
     "acorn": "6.0.2",
     "acorn-dynamic-import": "4.0.0",
     "acorn-walk": "6.1.0",
-    "google-closure-compiler": "20180910.1.0",
+    "google-closure-compiler": "20181008.0.0",
     "magic-string": "0.25.1",
     "temp-write": "3.4.0"
   },

package.json

@@ -29,7 +29,6 @@ import compiler from './compiler';
 import options from './options';
 import { preCompilation, createTransforms, deriveFromInputSource } from './transforms';
 import { Transform } from './types';
-import { logSource } from './debug';
 
 const readFile = promisify(fs.readFile);
 
@@ -48,7 +47,6 @@ const renderChunk = async (
   outputOptions: OutputOptions,
 ): Promise<{ code: string; map: RawSourceMap } | void> => {
   const code = await preCompilation(sourceCode, outputOptions, transforms);
-  logSource('transform', sourceCode, code);
   const [compileOptions, mapFile] = options(
     requestedCompileOptions,
     outputOptions,

src/index.ts

@@ -32,9 +32,7 @@ import {
   ExportClosureMapping,
 } from '../types';
 import MagicString from 'magic-string';
-const acornWalk = require('acorn-walk');
-const inject = require('acorn-dynamic-import/lib/walk').default;
-const walk = inject(acornWalk);
+const walk = require('acorn-dynamic-import/lib/walk').default(require('acorn-walk'));
 
 /**
  * This Transform will apply only if the Rollup configuration is for 'esm' output.
@@ -143,6 +141,7 @@ export default class ExportTransform extends Transform implements TransformInter
       const originalExportIdentifiers = Object.keys(originalExports);
 
       source.trimEnd();
+
       walk.ancestor(program, {
         // We inserted window scoped assignments for all the export statements during `preCompilation`
         // window['exportName'] = exportName;

src/transformers/exports.ts

@@ -14,47 +14,60 @@
  * limitations under the License.
  */
 
-import { Transform, ALL_IMPORT_DECLARATIONS, IMPORT_DECLARATION } from '../types';
-import { literalName } from './parsing-utilities';
+import { Transform } from '../types';
+import { literalName, importLocalNames } from './parsing-utilities';
 import { TransformSourceDescription } from 'rollup';
 import MagicString from 'magic-string';
+import { ImportDeclaration } from 'estree';
+const walk = require('acorn-dynamic-import/lib/walk').default(require('acorn-walk'));
 
-// TODO(KB): Need to generate externs for external members if we do not have an extern passed in.
-// Otherwise, advanced mode compilation will fail.
-// Likely this means moving scanning for external imports from `preCompilation` to the `derive` phase.
-// const HEADER = `/**
-// * @fileoverview Externs built via derived configuration from Rollup or input code.
-// * This extern contains the external import names, to prevent compilation failures.
-// * @externs
-// */
-// `;
+const HEADER = `/**
+* @fileoverview Externs built via derived configuration from Rollup or input code.
+* This extern contains the external import names, to prevent compilation failures.
+* @externs
+*/
+`;
 
 export default class ImportTransform extends Transform {
   private importedExternalsSyntax: { [key: string]: string } = {};
+  private importedExternalsLocalNames: Array<string> = [];
 
   /**
    * Rollup allows configuration for 'external' imports.
    * These are items that will not be bundled with the resulting code, but instead are expected
    * to already be available via `import` or other means.
    * @param source parsed from import statements, this is the source of a particular import statement.
-   * @return Promise<boolean> if the import is listed in the external list.
+   * @return boolean if the import is listed in the external list.
    */
-  private async isExternalImport(source: string): Promise<boolean> {
+  private isExternalImport(source: string): boolean {
     if (this.inputOptions.external === undefined) {
       return false;
     }
     if (Array.isArray(this.inputOptions.external)) {
       return this.inputOptions.external.includes(source);
     }
-    // TODO(KB): Restore if needed.
-    // if (typeof this.inputOptions.external === 'function') {
-    //   const configDrivenExternal = await this.inputOptions.external(source, parent, true);
-    //   return configDrivenExternal !== undefined && configDrivenExternal === true;
-    // }
 
     return false;
   }
 
+  /**
+   * Generate externs for local names of external imports.
+   * Otherwise, advanced mode compilation will fail since the reference is unknown.
+   * @return string representing content of generated extern.
+   */
+  public extern(): string {
+    if (this.importedExternalsLocalNames.length > 0) {
+      let extern = HEADER;
+      this.importedExternalsLocalNames.forEach(name => {
+        extern += `function ${name}(){};\n`;
+      });
+
+      return extern;
+    }
+
+    return '';
+  }
+
   /**
    * Before Closure Compiler modifies the source, we need to ensure external imports have been removed
    * since Closure will error out when it encounters them.
@@ -64,24 +77,25 @@ export default class ImportTransform extends Transform {
    * @return modified input source with external imports removed.
    */
   public async preCompilation(code: string): Promise<TransformSourceDescription> {
+    const self = this;
     const source = new MagicString(code);
-    const program = this.context.parse(code, { ranges: true });
-    const importNodes = program.body.filter(node => ALL_IMPORT_DECLARATIONS.includes(node.type));
+    const program = self.context.parse(code, { ranges: true });
 
-    for (const node of importNodes) {
-      switch (node.type) {
-        case IMPORT_DECLARATION:
-          const name = literalName(this.context, node.source);
-          if (await this.isExternalImport(name)) {
-            const range: [number, number] = node.range ? [node.range[0], node.range[1]] : [0, 0];
-            this.importedExternalsSyntax[name] = code.slice(range[0], range[1]);
-            source.remove(range[0], range[1]);
-          }
-          break;
-        default:
-          break;
-      }
-    }
+    walk.simple(program, {
+      async ImportDeclaration(node: ImportDeclaration) {
+        const name = literalName(self.context, node.source);
+
+        if (self.isExternalImport(name)) {
+          const range: [number, number] = node.range ? [node.range[0], node.range[1]] : [0, 0];
+          self.importedExternalsSyntax[name] = code.slice(range[0], range[1]);
+          source.remove(range[0], range[1]);
+
+          self.importedExternalsLocalNames = self.importedExternalsLocalNames.concat(
+            importLocalNames(self.context, node),
+          );
+        }
+      },
+    });
 
     return {
       code: source.toString(),

src/transformers/imports.ts

@@ -18,9 +18,7 @@ import { Transform } from '../types';
 import { TransformSourceDescription } from 'rollup';
 import MagicString from 'magic-string';
 import { ObjectExpression } from 'estree';
-const acornWalk = require('acorn-walk');
-const inject = require('acorn-dynamic-import/lib/walk').default;
-const walk = inject(acornWalk);
+const walk = require('acorn-dynamic-import/lib/walk').default(require('acorn-walk'));
 
 /**
  * Closure Compiler will not transform computed keys with literal values back to the literal value.

src/transformers/literal-computed-keys.ts

@@ -14,9 +14,21 @@
  * limitations under the License.
  */
 
-import { ExportNamedDeclaration, ExportDefaultDeclaration, Literal, SimpleLiteral } from 'estree';
+import {
+  ExportNamedDeclaration,
+  ExportDefaultDeclaration,
+  Literal,
+  SimpleLiteral,
+  ImportDeclaration,
+} from 'estree';
 import { PluginContext } from 'rollup';
-import { ExportNameToClosureMapping, ExportClosureMapping } from '../types';
+import {
+  ExportNameToClosureMapping,
+  ExportClosureMapping,
+  IMPORT_SPECIFIER,
+  IMPORT_NAMESPACE_SPECIFIER,
+  IMPORT_DEFAULT_SPECIFIER,
+} from '../types';
 
 type ExportDeclarationsWithFunctions = ExportNamedDeclaration | ExportDefaultDeclaration;
 
@@ -227,3 +239,26 @@ export function literalName(context: PluginContext, literal: Literal): string {
   const literalValue = (literal as SimpleLiteral).value;
   return typeof literalValue === 'string' ? literalValue : '';
 }
+
+export function importLocalNames(
+  context: PluginContext,
+  declaration: ImportDeclaration,
+): Array<string> {
+  const returnableSpecifiers: Array<string> = [];
+
+  if (declaration.specifiers) {
+    declaration.specifiers.forEach(specifier => {
+      switch (specifier.type) {
+        case IMPORT_SPECIFIER:
+        case IMPORT_NAMESPACE_SPECIFIER:
+        case IMPORT_DEFAULT_SPECIFIER:
+          returnableSpecifiers.push(specifier.local.name);
+          break;
+        default:
+          break;
+      }
+    });
+  }
+
+  return returnableSpecifiers;
+}

src/transformers/parsing-utilities.ts

@@ -26,10 +26,11 @@ import {
 
 // @see https://github.com/estree/estree/blob/master/es2015.md#imports
 export const IMPORT_DECLARATION = 'ImportDeclaration';
+export const DYNAMIC_IMPORT_DECLARATION = 'Import';
 export const IMPORT_SPECIFIER = 'ImportSpecifier';
 export const IMPORT_DEFAULT_SPECIFIER = 'ImportDefaultSpecifier';
 export const IMPORT_NAMESPACE_SPECIFIER = 'ImportNamespaceSpecifier';
-export const ALL_IMPORT_DECLARATIONS = [IMPORT_DECLARATION];
+export const ALL_IMPORT_DECLARATIONS = [IMPORT_DECLARATION, DYNAMIC_IMPORT_DECLARATION];
 
 // @see https://github.com/estree/estree/blob/master/es2015.md#exports
 export const EXPORT_NAMED_DECLARATION = 'ExportNamedDeclaration';

src/types.ts

@@ -64,6 +64,7 @@ function generate(
       const bundle = await rollup.rollup({
         input: fixtureLocation(category, name, format, optionKey, false),
         plugins: [compiler(closureFlags[optionKey])],
+        external: ['lodash'],
       });
 
       return {