Use default class property for @optional injected properties (#1467)

Currently the @optional tag overrides class properties defined with typescript-style default values:
```typescript
@Injectable()
class Ninja {
    @Inject(Weapon) @optional() weapon: Weapon = new Katana();

    ....
}
```
this currently sets the weapon property to `undefined` if no Weapon is present in the container. My proposed change allows does not override the default and results in the weapon property being set to new Katana().

(Default values in the constructor already work, but I would prefer not to add boilerplate constructors if this solution is possible.)

Co-authored-by: Podaru Dragos <[email protected]>

CHANGELOG.md

@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 ### Fixed
+property injection tagged as @optional no longer overrides default values with `undefined`. 
 
 ## [6.0.2]
 

src/resolution/instantiation.ts

@@ -68,7 +68,9 @@ function createInstanceWithInjections<T>(
   args.propertyRequests.forEach((r: interfaces.Request, index: number) => {
     const property = r.target.identifier;
     const injection = args.propertyInjections[index];
-    (instance as Record<string | symbol, unknown>)[property] = injection;
+    if (!r.target.isOptional() || injection !== undefined) {
+      (instance as Record<string | symbol, unknown>)[property] = injection;
+    }
   });
   return instance
 }

test/annotation/optional.test.ts

@@ -107,4 +107,49 @@ describe('@optional', () => {
 
   });
 
+  it("Should allow to set a default value for class property dependencies flagged as optional", () => {
+    @injectable()
+    class Katana {
+      public name: string;
+      public constructor() {
+        this.name = 'Katana';
+      }
+    }
+
+    @injectable()
+    class Shuriken {
+      public name: string;
+      public constructor() {
+        this.name = 'Shuriken';
+      }
+    }
+
+    @injectable()
+    class Ninja {
+      public name: string = "Ninja";
+      @inject("Katana") public katana?: Katana;
+      @inject("Shuriken") @optional() public shuriken: Shuriken = {
+        name: "DefaultShuriken",
+      };
+    }
+
+    const container = new Container();
+
+    container.bind<Katana>('Katana').to(Katana);
+    container.bind<Ninja>('Ninja').to(Ninja);
+
+    let ninja = container.get<Ninja>('Ninja');
+    expect(ninja.name).to.eql('Ninja');
+    expect(ninja.katana?.name).to.eql('Katana');
+    expect(ninja.shuriken.name).to.eql('DefaultShuriken');
+
+    container.bind<Shuriken>('Shuriken').to(Shuriken);
+
+    ninja = container.get<Ninja>('Ninja');
+    expect(ninja.name).to.eql('Ninja');
+    expect(ninja.katana?.name).to.eql('Katana');
+    expect(ninja.shuriken.name).to.eql('Shuriken');
+    }
+  );
+
 });

wiki/optional_dependencies.md

@@ -84,3 +84,13 @@ class Ninja {
     }
 }
 ```
+
+Or using properties injection: 
+```ts
+@injectable()
+class Ninja {
+    public name = "Ninja";
+    @inject("Katana") public katana: Katana;
+    @inject("Shuriken") @optional() public shuriken: Shuriken = { name: "DefaultShuriken" } // Default value!
+}
+```