feat: Throw if there are conflicting dependency names (#1155)

Adds validation to check that all dependencies have a unique name.

This follows on from [a discussion] with @zhibek about why we need to
turn off webpack's minification optimisation. TL;DR two dependency
classes can be minified to the same one-letter name, and since
dependencies are keyed by class name, this causes nasty runtime errors.

Lambda Wrapper will now be able to detect this situation and suggest
turning off minification, linking to the [readme note] of how to do
this for webpack.

As part of this, dependency classes will be deduplicated before being
instantiated. If a dependency is specified several times (unlikely but
possible) it will now be instantiated only once. Noting for completeness
in case we discover unexpected side effects down the line.

Jira: [ENG-2728]

[a discussion]: comicrelief/serverless-payments#614 (comment)
[readme note]: https://github.com/comicrelief/lambda-wrapper/tree/beta#notes

Author: seb-cr

src/core/DependencyInjection.ts

@@ -28,7 +28,38 @@ export default class DependencyInjection<TConfig extends LambdaWrapperConfig = a
     readonly event: any,
     readonly context: Context,
   ) {
-    const classes = Object.values(config.dependencies);
+    // get unique dependency classes -- a class may be included several times,
+    // but should be instantiated only once
+    const classes = Array.from(new Set(Object.values(config.dependencies)));
+
+    // guard against duplicate keys
+    const countByName = classes
+      .map((Constructor) => Constructor.name)
+      .reduce(
+        (counts, name) => ({ ...counts, [name]: (counts[name] || 0) + 1 }),
+        {} as Record<string, number>,
+      );
+    if (Object.values(countByName).some((count) => count > 1)) {
+      const duplicateNames = Object.entries(countByName)
+        .filter(([, count]) => count > 1)
+        .map(([name]) => name);
+
+      // if all class names are single-letter, they're probably minified -- in
+      // this case, give a hint about how to fix it
+      const action = duplicateNames.every((it) => it.length === 1)
+        ? "If you don't recognise the single-letter names listed above, your "
+          + "bundler may be minifying your code. You'll need to disable this "
+          + 'for Lambda Wrapper to work correctly. Please refer to the Notes '
+          + 'section of the Lambda Wrapper readme:\n\n'
+          + '  https://github.com/comicrelief/lambda-wrapper/tree/beta#notes'
+        : 'Please ensure that all dependency classes have a unique name.';
+
+      throw new Error(
+        `Dependency names are not unique: ${duplicateNames.join(', ')}\n\n${action}`,
+      );
+    }
+
+    // instantiate all dependencies
     this.dependencies = Object.fromEntries(
       classes.map((Constructor) => [Constructor.name, new Constructor(this)]),
     );

tests/mocks/dependencies.ts

@@ -0,0 +1,9 @@
+import { DependencyAwareClass } from '@/src';
+
+export class A extends DependencyAwareClass {}
+
+export class B extends DependencyAwareClass {}
+
+export class C extends DependencyAwareClass {}
+
+export class ServiceA extends DependencyAwareClass {}

tests/mocks/dependencies2.ts

@@ -0,0 +1,8 @@
+// Some more dependency classes, with the same name as those in dependencies.ts
+// so that we can test name conflicts.
+
+import { DependencyAwareClass } from '@/src';
+
+export class A extends DependencyAwareClass {}
+
+export class ServiceA extends DependencyAwareClass {}

tests/unit/core/DependencyInjection.spec.ts

@@ -1,11 +1,15 @@
-import { DependencyAwareClass, DependencyInjection } from '@/src';
+import { DependencyInjection } from '@/src';
 import { mockContext, mockEvent } from '@/tests/mocks/aws';
-
-class A extends DependencyAwareClass {}
-
-class B extends DependencyAwareClass {}
-
-class C extends DependencyAwareClass {}
+import {
+  A,
+  B,
+  C,
+  ServiceA,
+} from '@/tests/mocks/dependencies';
+import {
+  A as AClash,
+  ServiceA as ServiceAClash,
+} from '@/tests/mocks/dependencies2';
 
 describe('unit.core.DependencyInjection', () => {
   const mockConfig = {
@@ -17,6 +21,49 @@ describe('unit.core.DependencyInjection', () => {
 
   const di = new DependencyInjection(mockConfig, mockEvent, mockContext);
 
+  describe('constructor', () => {
+    describe('dependency conflicts', () => {
+      it('should throw if dependencies have conflicting names', () => {
+        const clashConfig = {
+          dependencies: {
+            ServiceA,
+            ServiceAClash,
+          },
+        };
+
+        expect(
+          () => new DependencyInjection(clashConfig, mockEvent, mockContext),
+        ).toThrowError('ensure that all dependency classes have a unique name');
+      });
+
+      it('should suggest turning off minification if names are single-letter', () => {
+        const clashConfig = {
+          dependencies: {
+            A,
+            AClash,
+          },
+        };
+
+        expect(
+          () => new DependencyInjection(clashConfig, mockEvent, mockContext),
+        ).toThrowError('your bundler may be minifying your code');
+      });
+
+      it('should not throw if the same dependency is included twice', () => {
+        const okayConfig = {
+          dependencies: {
+            A,
+            again: A,
+          },
+        };
+
+        expect(
+          () => new DependencyInjection(okayConfig, mockEvent, mockContext),
+        ).not.toThrow();
+      });
+    });
+  });
+
   describe('event', () => {
     it('should expose the event', () => {
       expect(di.event).toBe(mockEvent);