Skip to content

Commit

Permalink
fix(context): fix optional param injection for methods
Browse files Browse the repository at this point in the history
The following function has length of 0 as the param has a default value:
```
hello(@Inject('user', {optional: true}) user: string = 'Mary')
```

Before this fix, the injected value for `user` is not used. The PR uses
decorations to help decide the real number of params.
  • Loading branch information
raymondfeng committed Feb 23, 2018
1 parent a82babf commit 801a82d
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 5 deletions.
10 changes: 9 additions & 1 deletion packages/context/src/resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,15 @@ export function resolveInjectedArguments(
const injectedArgs = describeInjectedArguments(target, method);
const extraArgs = nonInjectedArgs || [];

const argLength = DecoratorFactory.getNumberOfParameters(target, method);
let argLength = DecoratorFactory.getNumberOfParameters(target, method);
if (argLength < injectedArgs.length) {
/**
* `Function.prototype.length` excludes the rest parameter and only includes
* parameters before the first one with a default value. For example,
* `hello(@inject('name') name: string)` gives 0 for argLength
*/
argLength = injectedArgs.length;
}

let nonInjectedIndex = 0;
return resolveList(new Array(argLength), (val, ix) => {
Expand Down
14 changes: 13 additions & 1 deletion packages/context/test/acceptance/method-level-bindings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ class InfoController {
return msg;
}

hello(@inject('user') user: string): string {
hello(
@inject('user', {optional: true})
user: string = 'Mary',
): string {
const msg = `Hello ${user}`;
debug(msg);
return msg;
Expand All @@ -41,6 +44,15 @@ describe('Context bindings - Injecting dependencies of method', () => {
expect(msg).to.eql('Hello John');
});

it('injects optional prototype method args', async () => {
ctx = new Context();
ctx.bind(INFO_CONTROLLER).toClass(InfoController);
const instance = await ctx.get(INFO_CONTROLLER);
// Invoke the `hello` method => Hello Mary
const msg = await invokeMethod(instance, 'hello', ctx);
expect(msg).to.eql('Hello Mary');
});

it('injects prototype method args with non-injected ones', async () => {
const instance = await ctx.get(INFO_CONTROLLER);
// Invoke the `hello` method => Hello John
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ describe('Bootstrapping with RestComponent', () => {
let server: RestServer;
before(givenAppWithUserDefinedSequence);

it('binds the `sequence` key to the user-defined sequence', async () => {
const binding = await server.get(RestBindings.SEQUENCE);
expect(binding.constructor.name).to.equal('UserDefinedSequence');
it('binds the `sequence` key to the user-defined sequence', () => {
// At this moment, the Sequence is not ready to be resolved
// as RestBindings.Http.CONTEXT is not bound
const binding = server.getBinding(RestBindings.SEQUENCE);
expect(binding.valueConstructor.name).to.equal('UserDefinedSequence');
});

async function givenAppWithUserDefinedSequence() {
Expand Down

0 comments on commit 801a82d

Please sign in to comment.