owja / ioc

:unicorn: lightweight (<1kb) inversion of control javascript library for dependency injection written in typescript
MIT License
287 stars 13 forks source link

`toValue` consider function given as a factory. #63

Closed liplum closed 1 year ago

liplum commented 1 year ago

Describe the bug The Container.bind(...).toValue(...) consider the a given function as a fatory.

To Reproduce

import { Container, token } from "@owja/ioc"
const TYPE = {
  myType: token("MyType")
}
const container = new Container()
container.bind(TYPE.myType).toValue((text) => {
  if (text === undefined) console.log("You cannot give undefined.")
  else console.log(text)
})
const myFunc = container.get(TYPE.myType)
myFunc("hello, world!")

the console output is

You cannot give undefined.
file:///test.js:11
myFunc("hello, world!")
^

TypeError: myFunc is not a function

Expected behavior I want to bind a Getter function, such as (options) => create(options) with toValue, however, the container considered it as a factory and tried to call it and retrieved the return value.

I don't think toValue() should detect its type under the hood, which made me confused. Instead, toFactory() is more suitable for this.

Environment (please complete the following information):

Additional context The workaround is to wrap the function with a new function.

container.bind(TYPE.myType).toValue(() => (text) => {
  if (text === undefined) console.log("You cannot give undefined.")
  else console.log(text)
})

But it makes the typing sucks in typescript. Because the token should be token<()=>(text:string)=>void>("MyType"), then the return value from container.get(...) is typed by ()=>(text:string)=>void but (text:string)=>void is actually expected.

liplum commented 1 year ago

After I checked the source code, I found the factory checking in Container.get.

get<T, U extends Array<unknown> = never>(
    token: Token<T, U> | MaybeToken<T>,
    tags: symbol[] | symbol = [],
    target?: unknown,
    injectedArgs: Array<unknown> = [],
): T {
    const item = <Item<T> | undefined>this._registry.get(getType(token));
    if (item === undefined || item.injected === undefined) throw `nothing bound to ${stringifyToken(token)}`;
    const value = isFactory(item.injected)
        ? !item.singleton
            ? item.injected(...injectedArgs)
            : (item.cache = item.cache || item.injected())
        : item.injected;
    const tagsArr = valueOrArrayToArray(tags);
    if (tagsArr.indexOf(NOPLUGINS) === -1)
        item.plugins.concat(this._plugins).forEach((plugin) => {
            plugin(value, target, tagsArr, token, this);
        });
    return value;
}

the isFactory only checks whether the injected is a function no matter it comes from toValue or toFactory.

const isFactory = <T>(i: Injected<T>): i is Factory<T> => typeof i === "function";
hbroer commented 1 year ago

@liplum i published a new version (2.0.0-alpha.7). That should fix the bug.

liplum commented 1 year ago

Yes, it works.