Further cleanup for import transform

Author: kristoferbaxter

src/parsing/import-specifiers.ts

@@ -0,0 +1,73 @@
+/**
+ * Copyright 2020 The AMP HTML Authors. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS-IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { IMPORT_SPECIFIER, IMPORT_NAMESPACE_SPECIFIER, IMPORT_DEFAULT_SPECIFIER } from '../types';
+import { ImportSpecifier, ImportDefaultSpecifier, ImportNamespaceSpecifier } from 'estree';
+
+export interface Specifiers {
+  default: string | null;
+  specific: Array<string>;
+  local: Array<string>;
+}
+
+export function Specifiers(
+  specifiers: Array<ImportSpecifier | ImportDefaultSpecifier | ImportNamespaceSpecifier>,
+): Specifiers {
+  let defaultName: Specifiers['default'] = null;
+  let specificNames: Specifiers['specific'] = [];
+  let localNames: Specifiers['local'] = [];
+
+  for (const specifier of specifiers) {
+    localNames.push(specifier.local.name);
+
+    switch (specifier.type) {
+      case IMPORT_SPECIFIER:
+      case IMPORT_NAMESPACE_SPECIFIER:
+        const { name: local } = (specifier as ImportSpecifier).local;
+        const { name: imported } = (specifier as ImportSpecifier).imported;
+
+        if (local === imported) {
+          specificNames.push(local);
+        } else {
+          specificNames.push(`${imported} as ${local}`);
+        }
+        break;
+      case IMPORT_DEFAULT_SPECIFIER:
+        defaultName = specifier.local.name;
+        break;
+    }
+  }
+
+  return {
+    default: defaultName,
+    specific: specificNames,
+    local: localNames,
+  };
+}
+
+export function FormatSpecifiers(specifiers: Specifiers, name: string): string {
+  let formatted: string = 'import ';
+
+  if (specifiers.default !== null) {
+    formatted += specifiers.default;
+  }
+  if (specifiers.specific.length > 0) {
+    formatted += `${specifiers.default !== null ? ',' : ''}{${specifiers.specific.join(',')}}`;
+  }
+  formatted += ` from '${name}';`;
+
+  return formatted;
+}

src/transformers/imports.ts

@@ -14,17 +14,12 @@
  * limitations under the License.
  */
 
-import {
-  Transform,
-  Range,
-  IMPORT_SPECIFIER,
-  IMPORT_NAMESPACE_SPECIFIER,
-  IMPORT_DEFAULT_SPECIFIER,
-} from '../types';
+import { Transform, Range } from '../types';
 import { literalName } from '../parsing/literal-name';
+import { FormatSpecifiers, Specifiers } from '../parsing/import-specifiers';
 import { TransformSourceDescription } from 'rollup';
 import MagicString from 'magic-string';
-import { ImportDeclaration, Identifier, ImportSpecifier } from 'estree';
+import { ImportDeclaration, Identifier } from 'estree';
 import { parse, walk } from '../acorn';
 
 const DYNAMIC_IMPORT_KEYWORD = 'import';
@@ -42,19 +37,6 @@ interface RangedImport {
   range: Range;
 }
 
-const VALID_SPECIFIERS = [IMPORT_SPECIFIER, IMPORT_NAMESPACE_SPECIFIER, IMPORT_DEFAULT_SPECIFIER];
-function importLocalNames(declaration: ImportDeclaration): Array<string> {
-  const returnableSpecifiers: Array<string> = [];
-
-  for (const specifier of declaration.specifiers || []) {
-    if (VALID_SPECIFIERS.includes(specifier.type)) {
-      returnableSpecifiers.push(specifier.local.name);
-    }
-  }
-
-  return returnableSpecifiers;
-}
-
 export default class ImportTransform extends Transform {
   private importedExternalsSyntax: { [key: string]: string } = {};
   private importedExternalsLocalNames: Array<string> = [];
@@ -105,43 +87,11 @@ window['${DYNAMIC_IMPORT_REPLACEMENT}'] = ${DYNAMIC_IMPORT_REPLACEMENT};`;
       async ImportDeclaration(node: ImportDeclaration) {
         const name = literalName(node.source);
         const range: Range = node.range as Range;
+        const specifiers: Specifiers = Specifiers(node.specifiers);
 
-        let defaultSpecifier: string | null = null;
-        const specificSpecifiers: Array<string> = [];
-        for (const specifier of node.specifiers) {
-          switch (specifier.type) {
-            case IMPORT_SPECIFIER:
-            case IMPORT_NAMESPACE_SPECIFIER:
-              const { name: local } = (specifier as ImportSpecifier).local;
-              const { name: imported } = (specifier as ImportSpecifier).imported;
-              specificSpecifiers.push(local === imported ? local : `${imported} as ${local}`);
-              break;
-            case IMPORT_DEFAULT_SPECIFIER:
-              defaultSpecifier = specifier.local.name;
-              break;
-          }
-        }
-
-        const multipleSpecifiers = specificSpecifiers.length > 0;
-        if (defaultSpecifier !== null) {
-          if (multipleSpecifiers) {
-            self.importedExternalsSyntax[
-              name
-            ] = `import ${defaultSpecifier},{${specificSpecifiers.join(',')}} from '${name}';`;
-          } else {
-            self.importedExternalsSyntax[name] = `import ${defaultSpecifier} from '${name}';`;
-          }
-        } else if (multipleSpecifiers) {
-          self.importedExternalsSyntax[name] = `import {${specificSpecifiers.join(
-            ',',
-          )}} from '${name}';`;
-        }
-
+        self.importedExternalsSyntax[name] = FormatSpecifiers(specifiers, name);
+        self.importedExternalsLocalNames.push(...specifiers.local);
         source.remove(...range);
-
-        self.importedExternalsLocalNames = self.importedExternalsLocalNames.concat(
-          importLocalNames(node),
-        );
       },
       Import(node: RangedImport) {
         const [start, end] = node.range;