nikku / didi Goto Github PK

Describe the Bug

Hi,
I noticed that minified output sometimes inlines classes I pass to didi.
In this case, it generates an unexpected dependency called class.

Steps to Reproduce

1. parse annotations with a class without name.

        expect(parseAnnotations(class{})).to.eql([ ]);

2. expect no dependencies
3. parseAnnotations in this example returns [ "class" ]

      AssertionError: expected [ 'class' ] to deeply equal []
      + expected - actual

      -[
      -  "class"
      -]
      +[]

Expected Behavior

Parsing this class should return an empty array or an array without class

Environment

• Host (Browser/Node version), if applicable: Node 16.17.0
• OS: linux
• Library version: 8.0.1

Possible solution

It seems like this happens because didi determines that a class is a class by checking if it starts with class .
In the inlined case, the class can start with class without an extra space

Would it be ok to change the heuristic to check for class or class{? I can create a PR with a test if that's ok.
I.e. a solution like the following would solve this issue:

var CLASS_PATTERN = /^class[ {]/;` 

I've created #18 which implements this solution

This is handy if components are built in a way that they can handle the non-existance of a service.

At the moment, there is no way to do this and an error will be thrown (and has to be caught) _all the time.

I would expect this to work:

var di = require('didi');

class Car {
    start() {
        console.log('success!');
    }
}

var module = {
    'car': ['type', Car],
};

var injector = new di.Injector([module]);
injector.invoke(['car', function (car) {
    car.start();
}]);

But what I get is this error:

node_modules\didi\dist\didi.js:125
  return match[1] && match[1].split(',').map(function (arg) {
              ^

TypeError: Cannot read property '1' of null
    at parse (c:\z\github\nikku\tmp\node_modules\didi\dist\didi.js:125:15)

The problem seems to be with annotation.js, where the toString of the class is expected to include this regex:

/^function\s*[^(]*\(\s*([^)]*)\)/m;

But the toString is the class definition above and thus does not comply to the regex.

Describe the Bug

The context and locals parameters of Injector#invoke are optional. The Type declaration doesn't reflect that:

invoke<T>(func: (...args: unknown[]) => T, context: InjectionContext, locals: LocalsMap): T;

Expected Behavior

The declaration should be:

invoke<T>(func: (...args: unknown[]) => T, context?: InjectionContext, locals?: LocalsMap): T;

Would be great to support unpacking or destructured objects for function parameters.
Working on a PR here with update to the readme.

// destructured object parameter wont require any annotations. 
function createEngine({power}) { ... }

//then in module config can take the simple route as well since function params are parsed and $inject is automatically added
const carModule = {
  engine: ['factory', createEngine ],
  power:  ['value', { horses: 400 }]
};

Let me know what you think. Would this be something your interested in merging in. Plan on finishing tests today and will convert PR off draft once done.
Great project, thanks.

Running npm i in the repository errors on windows.

$ git clone [email protected]:nikku/didi.git
$ cd didi
$ npm i

> [email protected] prepare C:\z\utrechtjs\didi
> run-s bundle


> [email protected] bundle C:\z\utrechtjs\didi
> NODE_ENV=production rollup -c

'NODE_ENV' is not recognized as an internal or external command,
operable program or batch file.
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] bundle: `NODE_ENV=production rollup -c`

Is the bundle step important to run on as "prepare" script? Can we maybe use cross-env to ensure it works on windows and linux?

hi

nice library to apply DI, for plain js development

Issue:- on applying minify to js bundle, it seem not able to resolve the module, any pointer, its actually not matching the name in constructor of class from look up, any pointer?

thanks
Vimal

I would expect this code example to work:

var di = require('didi');

class Car {
    start() {
        console.log('success!');
    }
}

Car.$inject = [];

var module = {
    'car': ['type', Car],
};


var injector = new di.Injector([module]);
injector.invoke(['car', function (car) {
    car.start();
}]);

Instead i get this error:

didi\dist\didi.js:316
    return fn.apply(context, dependencies);
              ^

TypeError: Class constructor Car cannot be invoked without 'new'

Which is pretty much self explanatory, apparently classes are not supported (yet).

This is contradicted in the readme however:

// if an object asks for 'car', the injector will call new Car(...) to produce it
'car': ['type', Car],

Describe the Bug

In a project using didi for dependency injection, the new version with types included causes TS errors. It seems that the errors are so terrible to TypeScript that it actually will actually refuse to ignore the errors even with node_modules excluded.

The errors are the following three:

node_modules/didi/dist/types/lib/types.d.ts:56:3 - error TS2411: Property '__exports__' of type 'string[] | undefined' is not assignable to string index type 'string[] | ModuleDeclaration[] | ["value", any] | ["value", any, "private"] | ["type", TypeDeclaration<any>] | ["type", TypeDeclaration<any>, "private"] | [...] | [...]'.

56   __exports__?: string[],
     ~~~~~~~~~~~

node_modules/didi/dist/types/lib/types.d.ts:57:3 - error TS2411: Property '__modules__' of type 'ModuleDeclaration[] | undefined' is not assignable to string index type 'string[] | ModuleDeclaration[] | ["value", any] | ["value", any, "private"] | ["type", TypeDeclaration<any>] | ["type", TypeDeclaration<any>, "private"] | [...] | [...]'.

57   __modules__?: ModuleDeclaration[],
     ~~~~~~~~~~~

node_modules/didi/dist/types/lib/types.d.ts:58:38 - error TS8020: JSDoc types can only be used inside documentation comments.

58   [name: string]: ServiceDeclaration<?> | ModuleDeclaration[] | string[]

Steps to Reproduce

1. Setup a new project
2. Install [email protected]
3. Setup a tsconfig with "module": "es2020" and "strict": true (possibly "target": "es2020" and "moduleResolution": "node" as well). Exclude node_modules.
4. Create a main.ts where "didi" is dynamically imported with const { Injector } = await import("didi");.
5. Run tsc build
6. Errors should appear in build.

Expected Behavior

Library TS definitions should not contain errors.

It seems that

1. The ModuleDeclaration arrays should not be optional.
2. The ? in the ServiceDeclaration's type parameter seems to be an obvious JSDoc oversight.

Environment

• Ubuntu 18.04 WSL2 env