Consider using dependency-tree instead of precinct

dougmoscrop / serverless-plugin-include-dependencies

MIT License

184 stars 40 forks source link

Consider using dependency-tree instead of precinct #2

Closed mrjoelkemp closed 7 years ago

mrjoelkemp commented 7 years ago

Hey! Thanks for using my precinct module!

I was reading through your module's code and thought https://github.com/dependents/node-dependency-tree/ would be helpful for your get-dependency-list file. It includes partial to filename resolution and uses precinct under the hood.

Cheers!

dougmoscrop commented 7 years ago

Yeah I originally had a bug when using that module but I will revisit it!

mrjoelkemp commented 7 years ago

Glad to hear. Happy to address any bugs that you find. Feel free to open an issue. Thanks in advance

dougmoscrop commented 7 years ago

Hey so I pushed a branch that attempts to use dependency-tree: https://github.com/dougmoscrop/serverless-plugin-include-dependencies/tree/deptree

There's two problems:

The first is that really this project resolves packages in order to better support things that don't have a 'main', deptree seems to struggle with this?
Second, even for things that do have main entries, deptree seems to not be able to find them. I've added a console.log in the code in this branch that spits out the missing items, and it's weird because I can find them using resolvePkg / resolve

mrjoelkemp commented 7 years ago

Thanks for giving it a shot @dougmoscrop! Here's how commonjs modules are resolved: https://github.com/dependents/node-filing-cabinet/blob/21e44d4d96d4f34a9db6a0c6bbad53a35817635d/index.js#L155. We're adding the file's directory as a resolution path (since we'd otherwise use the executing script's current directory). We then rely on https://github.com/substack/node-resolve (not sure if upgrading that package would help the issues you're facing) for the resolution.

I'd be open to any fixes to cjs resolution that would resolve your issues. Otherwise, it seems like you already have a path forward without dependency-tree.

Thanks again for looking into this!