From 361c7a3e282fed538b98cbe848ab7917bb230fac Mon Sep 17 00:00:00 2001
From: Cas Haaijman <94356063+haaijman-imagem@users.noreply.github.com>
Date: Thu, 23 Jun 2022 08:49:51 +0200
Subject: [PATCH] Use default class property for @optional injected properties

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.)
---
 src/resolution/instantiation.ts  |  4 ++-
 test/annotation/optional.test.ts | 45 ++++++++++++++++++++++++++++++++
 wiki/optional_dependencies.md    | 10 +++++++
 3 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/src/resolution/instantiation.ts b/src/resolution/instantiation.ts
index 8e4537176..66b7976b4 100644
--- a/src/resolution/instantiation.ts
+++ b/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
 }
diff --git a/test/annotation/optional.test.ts b/test/annotation/optional.test.ts
index 1230e813b..d2e610b65 100644
--- a/test/annotation/optional.test.ts
+++ b/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');
+    }
+  );
+
 });
\ No newline at end of file
diff --git a/wiki/optional_dependencies.md b/wiki/optional_dependencies.md
index 10af62a26..ab202eb5a 100644
--- a/wiki/optional_dependencies.md
+++ b/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!
+}
+```
\ No newline at end of file