fix(imports): fix a bug where registering the same implementation mul…

…tiple times will generate multiple imports
wessberg · May 21, 2021 · f8c388b · f8c388b
1 parent 265ac93
commit f8c388b
Show file tree

Hide file tree

Showing 2 changed files with 71 additions and 0 deletions.
diff --git a/src/transformer/before/before-transformer.ts b/src/transformer/before/before-transformer.ts
@@ -21,12 +21,33 @@ function transformSourceFile(
 ): TS.SourceFile {
   const requiredImportedSymbolSet = new Set<ImportedSymbol>();
 
+  /**
+   * An optimization in which every imported symbol is converted into
+   * a string that can be matched against directly to guard against
+   * duplicates
+   */
+  const requiredImportedSymbolSetFlags = new Set<string>();
+
   context.sourceFileToAddTslibDefinition.set(sourceFile.fileName, false);
   context.sourceFileToRequiredImportedSymbolSet.set(
     sourceFile.fileName,
     requiredImportedSymbolSet
   );
 
+  const computeImportedSymbolFlag = (symbol: ImportedSymbol): string =>
+    [
+      "name",
+      "propertyName",
+      "moduleSpecifier",
+      "isNamespaceImport",
+      "isDefaultImport",
+    ]
+      .map(
+        (property) =>
+          `${property}:${symbol[property as keyof ImportedSymbol] ?? false}`
+      )
+      .join("|");
+
   const visitorOptions: Omit<
     BeforeVisitorOptions<TS.Node>,
     "node" | "sourceFile"
@@ -38,6 +59,12 @@ function transformSourceFile(
     },
 
     requireImportedSymbol: (importedSymbol: ImportedSymbol): void => {
+      // Guard against duplicates and compute a string so we can do
+      // constant time lookups to compare against existing symbols
+      const flag = computeImportedSymbolFlag(importedSymbol);
+      if (requiredImportedSymbolSetFlags.has(flag)) return;
+      requiredImportedSymbolSetFlags.add(flag);
+
       requiredImportedSymbolSet.add(importedSymbol);
     },
 

diff --git a/test/container.test.ts b/test/container.test.ts
@@ -156,6 +156,50 @@ test(
   }
 );
 
+test(
+  "Won't include imports multiple times when the same implementation is registered multiple times. #1",
+  withTypeScript,
+  (t, { typescript }) => {
+    const bundle = generateTransformerResult(
+      [
+        {
+          entry: true,
+          fileName: "index.ts",
+          text: `
+				import {DIContainer} from "@wessberg/di";
+				import {IFoo, Foo} from "./foo";
+				
+				const container = new DIContainer();
+				container.registerSingleton<IFoo, Foo>();
+				container.registerSingleton<IFoo, Foo>();
+			`,
+        },
+        {
+          entry: false,
+          fileName: "foo.ts",
+          text: `
+				export interface IFoo {}
+				export class Foo implements IFoo {}
+			`,
+        },
+      ],
+      { typescript }
+    );
+    const file = bundle.find(({ fileName }) => fileName.includes("index.js"))!;
+
+    t.deepEqual(
+      formatCode(file.text),
+      formatCode(`\
+			import { Foo } from "./foo";
+      import { DIContainer } from "@wessberg/di";
+      const container = new DIContainer();
+      container.registerSingleton(undefined, { identifier: "IFoo", implementation: Foo });
+      container.registerSingleton(undefined, { identifier: "IFoo", implementation: Foo });
+			`)
+    );
+  }
+);
+
 test(
   "Supports custom implementation functions. #1",
   withTypeScript,