-
Notifications
You must be signed in to change notification settings - Fork 412
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use webpack context config property as working directory for finding the handler functions #325
Use webpack context config property as working directory for finding the handler functions #325
Conversation
I reiterated the PR. There is one case that might break by moving the config evaluation before the entries object generation: If the webpack config extends the entries array dynamically (e.g. to add static entries for parallel modules after the generated ones) or uses its contents to do other stuff. This will not work anymore, because the entries are now populated after the webpack config file has been required. I'm currently thinking about a solution for that. It must somehow be possible to have the context property set, but with creating the entries array upfront. An idea I have here is, to do a multi-pass require of the webpack config in case the context property has been set. So I'm thinking of the following algorithm: (1) Require the webpack config (without using the require cache) If you don't mind I'll do a test implementation and finally submit that to this PR. |
Hi @HyperBrain ... I understand your points. I'd be quite happy for you to make any changes you see fit. Thanks for stepping in on this btw 👍 |
Hi @BrettStrongEH , no problem. As soon as I found a viable and stable solution I will update it and integrate it into one of the next releases. I really like the idea of utilizing the context configuration 😄 . |
Thanks so much @HyperBrain ... looking forward to seeing how you go. |
Info: I'll come back to this one soon after the 5.1.0 release. |
Is this feature still on the roadmap at all? Would really be helpful in projects where the serverless configuration lives in a subdirectory of the main project. |
Yes it is, but the PR has to be reworked, because in the way it is implemented right now, it introduces breaking changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, may you please address the merge conflicts? Thanks.
What did you implement:
Closes #324
Currently if you use the auto detection feature (i.e. entry: slsw.lib.entries ), and your handler function paths point to a parent directory, the plugin cannot find them, which results in an empty slsw.lib.entries object which causes webpack to throw an error at run time
You can however reference parent paths in custom entry webpack config, however this requires manual updating when you need to add \ or remove functions in the serverless.yml template which defeats purpose of auto detection feature of this plugin.
My natural next step was to add the context property to the webpack config file in the hope that this plugin would honor that as the working directory when trying to find the funtion handler modules.
However the plugin did not into account the working directory configured by webpack's "context" property value (i.e. in the webpack.config.js file), and so that also ultimately failed at webpack runtime for same reasons as above (i.e. empty entry property).
So this PR will allow the end user to add the "context" webpack config property to the webpack.config.js file, and then the serverless-webpack plugin will use that value as the working directory when searching for the function handler modules. Everything else is untouched.
This would allow you to have multiple services with functions handlers that reference the same centralized code base which may exist outside of each serverless project directory.
Sample structure
How did you implement it:
Only 2 small changes ...
1 ...
I changed the lib\validate.js code that finds the files using glob.sync to use the webpack.config "context" value (if one exists) as the cwd value so that it can find the handler function path\file in that directory.
2 ...
Also in lib\index.js I had to make sure that the initial value of the slsw.lib.entries is some other than an empty object so that we can determine the user opted in to auto detection using entry: slsw.lib.entries.
There is no other custom serverless-plugin properties or configs needed (other than the standard webpack config property "context" which needs to be added to the webpack config file).
How can we verify it:
Todos:
Is this ready for review?: YES
Is it a breaking change?: NO