Build issues

[ehamai]

Hi there,

I'm in using your library because I really like the way you chose to inject data. However I'm having some issues getting this to build because there appear to be a lot of conflicts or missing *.d.ts references. You can see the latest commit that I made to add your library here. Here are some of the errors I'm seeing:

node_modules/ts-dependency-injection/lib/Injector.ts(1,1): error TS6053: File 'node_modules/ts-dependency-injection/node_modules/reflect-metadata/reflect-metadata.d.ts' not found.
node_modules/ts-dependency-injection/typings/express/express.d.ts(1075,5): error TS2300: Duplicate identifier 'export='.
node_modules/ts-dependency-injection/typings/mocha/mocha.d.ts(8,5): error TS2300: Duplicate identifier 'slow'.

I think the missing d.ts files may just be missing references in your package, and the conflicts are due to the fact that I'm using the same libraries (like mocha) as you. I think these issues would go away if your package only installed compiled code. Would it be possible to add that support or was the expectation that users would build the library on their own and add it to the project manually?

The other build issue I'm seeing is @Deps.Injection(IFSService) (see file.ts) doesn't seem to recognize my interface IFSService (I get an error that says "Cannot find name 'IFSService'"). Is that a bug or am I just using this incorrectly?

Sorry to bother you with build issues, but I'm fairly new to NodeJS so I appreciate any help you can provide. Thanks!

[toms-dev]

Hello! Thank you for using my lib :) The issue you are encountering is due to the fact that the typings of Reflect-Metadata are provided within its npm module, and not distributed through TSD or Typings. It's then missing because I forgot the step 'tsd link' in the build, which would automatically add the 'reflect-metadata.d.ts' file to the typings directory. I'm fixing this right away!

Yes, the error of duplicate identifiers does come from this shared ambiant typing dependency. As you correctly guessed, I originally wanted to distribute only the source of the lib, because it felt somewhat more elegant to me, but it also seems to cause many issues of this kind. I've been looking around quite a bit for best (better?) practices and it seems that distributing compiled code with matching definition file is actually the way to go, as it can bundle ambiant dependencies into a module to prevent version conflicts/collisions. Distributing compiled code would actually also fix the missing 'reflect-metadata' (as it would be automatically bundled inside), but as this may take I some more time than just fixing the build in package.json, I'm opening an issue and will try fix this asap! Thanks for telling me about those problems!

[toms-dev]

Ok, the problems does not seems to come from a missing instruction in the build, but rather from the way paths in /// <reference ...> are resolved (from project root, and not the current source file, which seems legit !). I'm investigating for a quick solution ;)

By the way, the issue with Interfaces is that TypeScript does not emit any actual Javascript object representing them, it is only used to enforce a structure on objects during compilation. Thus at runtime, there is nothing to match the object against when resolving the dependencies. I will still take some more time to see if it's possible to have TypeScript emitting something when compiling, but I think it's not the case... Opening an issue for this too!

[ehamai]

Thanks for looking into this so quickly! :) Personally I'd prefer the compiled version but I understand that it might be difficult to create.

Also for the interfaces issue, I kind of figured it had something to do with the fact that interfaces don't generate JS, but I wasn't sure. I'm not sure if it's possible to fix or not, but I think that it would be a pretty common use case for people to want to resolve to interfaces instead of classes. Just my opinion. Anyway thanks again.

[toms-dev]

Ok, I just published the new version on npm (version 1.2.0), which now contains the compiled code! Tell me if your problems are gone with this release ;) EDIT: ok looks like I messed up something, wait a bit x) EDIT2: ok, looks fine so far.

I totally agree with you, I will definitely look into the Interface issue, this is a quite common use case, so it would be a shame to miss it!

[ehamai]

Thanks Tom! I changed my code to use a base class instead of an interface and rebuilt with the new version but still had a compile failure where inject.d.ts was failing to resolve reflect-metadata.d.ts. I removed the reference in that file to reflect-metadata.d.ts and was able to compile.

Unfortunately after rerunning my test, it seems to hang on the call to "resolve". Here's the commit if you'd like to see the change I made. Am I doing this wrong?

[NickTsitlakidis]

Hello, I just had the same issue with the reference path and fixed it by adding the reflect metadata typing through their github repo.

Please check my pull request.

[toms-dev]

I merged the pull request and the latest version is now published on NPM. Please keep me updated if you encounter any issue!