serverless-webpack icon indicating copy to clipboard operation
serverless-webpack copied to clipboard

Use webpack context config property as working directory for finding the handler functions

Open Bostrong opened this issue 6 years ago • 7 comments

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

  • srcRoot
    • serverlessProjA
      • serverless.yml --> handler: "functions/projA/index.js"
      • webpack.config.js --> context: "../webAPISrc"
    • serverlessProjB
      • serverless.yml --> handler: "functions/projB/index.js"
      • webpack.config.js --> context: "../webAPISrc"
    • webAPISrc
      • functions
        • projA
          • index.js
        • projB
          • index.js

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:

  • [ ] Write tests
  • [ ] Write documentation
  • [ ] Fix linting errors
  • [ ] Make sure code coverage hasn't dropped
  • [ ] Provide verification config / commands / resources
  • [ ] Enable "Allow edits from maintainers" for this PR
  • [ ] Update the messages below

Is this ready for review?: YES Is it a breaking change?: NO

Bostrong avatar Feb 21 '18 02:02 Bostrong

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) (2) Check if the context property has been set (3) Build the entries object (which is empty at the very start again) - use the context property. (4) Finally require the webpack configuration as was

If you don't mind I'll do a test implementation and finally submit that to this PR.

HyperBrain avatar Feb 27 '18 12:02 HyperBrain

Hi @HyperBrain ...

I understand your points. To be honest ... I just forked it and implemented the fix so that I could start using it immediately. I did not take a lot of care regression testing any other test cases sorry.

I'd be quite happy for you to make any changes you see fit.

Thanks for stepping in on this btw 👍

BrettStrongEH avatar Feb 28 '18 03:02 BrettStrongEH

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 😄 .

HyperBrain avatar Feb 28 '18 09:02 HyperBrain

Thanks so much @HyperBrain ... looking forward to seeing how you go.

BrettStrongEH avatar Feb 28 '18 21:02 BrettStrongEH

Info: I'll come back to this one soon after the 5.1.0 release.

HyperBrain avatar Mar 14 '18 16:03 HyperBrain

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.

SebastianEdwards avatar Jun 03 '18 02:06 SebastianEdwards

Yes it is, but the PR has to be reworked, because in the way it is implemented right now, it introduces breaking changes.

HyperBrain avatar Jun 04 '18 19:06 HyperBrain