search

dougmoscrop / serverless-plugin-include-dependencies

MIT License
184 stars 40 forks source link

feat: allow local dependencies #24

Closed joscha closed 5 years ago

joscha commented 5 years ago

This change allows us to use local dependencies. They can be used when multiple source roots are defined via NODE_PATH for example. Currently, the plugin incorrectly assumes that anything not starting with ./ or not resolved against the cwd is a module dependency.

references https://github.com/dougmoscrop/serverless-plugin-include-dependencies/issues/14 and https://github.com/dougmoscrop/serverless-plugin-include-dependencies/issues/23

Happy to add an example and tests if this change is accepted.

dougmoscrop commented 5 years ago

Thanks, I had to do a bit more dancing around to get the tests passing and my manual testing to work; I also cleaned up the code. I think this change is no longer necessary - can you give 3.2.0 a try and let me know?

joscha commented 5 years ago

3.2.0 doesn't work for me, fails with:

  Error --------------------------------------------------

  [serverless-plugin-include-dependencies]: Could not find my-package

     For debugging logs, run again after setting the "SLS_DEBUG=*" environment variable.

  Get Support --------------------------------------------
     Docs:          docs.serverless.com
     Bugs:          github.com/serverless/serverless/issues
     Issues:        forum.serverless.com

  Your Environment Information -----------------------------
     OS:                     darwin
     Node Version:           10.13.0
     Serverless Version:     1.33.2

which is the same error I get with pre-3.2.0 w/o my fix. I can potentially create a test with a fixture that shows the problem if that helps?

dougmoscrop commented 5 years ago

yes, please that would be helpful

joscha commented 5 years ago

@dougmoscrop pushed a commit with a test fixture and the fix.

dougmoscrop commented 5 years ago

Ok I see what's going on here, and if you say it works for you then I'm gonna release it, I'm not entirely convinced that this will not have potential corner cases but using NODE_PATH seems like it's already surrounded in warnings/discouragement so I feel like it's a reasonable at-your-own risk kind of thing.

joscha commented 5 years ago

I totally agree. Thank you!

On Fri., 7 Dec. 2018, 03:20 Doug Moscrop <notifications@github.com wrote:

Ok I see what's going on here, and if you say it works for you then I'm gonna release it, I'm not entirely convinced that this will not have potential corner cases but using NODE_PATH seems like it's already surrounded in warnings/discouragement so I feel like it's a reasonable at-your-own risk kind of thing.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/dougmoscrop/serverless-plugin-include-dependencies/pull/24#issuecomment-444930941, or mute the thread https://github.com/notifications/unsubscribe-auth/AALehj3wBfnEu_aX_uIRkmK8XBI5Unulks5u2UPDgaJpZM4Y47h- .

joscha commented 5 years ago

Works like a charm, thanks @dougmoscrop