serverless-heaven / serverless-webpack

Serverless plugin to bundle your lambdas with Webpack
MIT License
1.72k stars 417 forks source link

Individually packaging with multi-modules entry does not work #397

Open geovanisouza92 opened 6 years ago

geovanisouza92 commented 6 years ago

This is a Bug Report

Description

I'm customizing entries on webpack to inject common funcionality, like source-map support and tracing modules. If I package functions individually, it does not work.

I tracked the issue to this line where the entry value is an array, and the path module is applied incorrectly.

If I understood correctly:

So, we could just use the key that is the entrypoints based on serverless "handler" configurations, and the problem will be solved.

For bug reports:

When I enabled individual packaging and use multiple modules for each entry, it throw the error "Path must be a string. Received [ './source-map-install.js', (redacted) ]"

The commands sls invoke local and sls webpack working fine.

serverless.yml

``` service: name: (redacted) plugins: - serverless-webpack - serverless-offline provider: name: aws runtime: nodejs6.10 functions: ping: handler: src/functions/ping.handler events: - http: method: get path: ping # (there are more functions here redacted) router: handler: src/functions/router.handler events: - http: method: post path: /new api: handler: src/functions/api.handler events: - http: method: post path: /api custom: stage: ${opt:stage, self:provider.stage} serverless-offline: port: 4000 stage: ${self:custom.stage} package: individually: true ```

webpack.config.js

``` const path = require("path"); const slsw = require("serverless-webpack"); const { stage } = slsw.lib.serverless.service.custom; const tracingModule = stage === "dev" ? "./src/tracing/jaeger/index.ts" : "./src/tracing/xray/index.ts"; const entries = Object.keys(slsw.lib.entries).reduce((entries, key) => { entries[key] = ["./source-map-install.js", tracingModule, slsw.lib.entries[key]]; return entries; }, {}); module.exports = { entry: entries, devtool: "source-map", externals: ["aws-sdk"], resolve: { extensions: [".js", ".jsx", ".json", ".ts", ".tsx"], }, output: { libraryTarget: "commonjs", path: path.join(__dirname, ".webpack"), filename: "[name].js", }, target: "node", module: { loaders: [{ test: /\.ts(x?)$/, loader: "ts-loader" }], }, }; ```
``` Stack Trace -------------------------------------------- TypeError: Path must be a string. Received [ './source-map-install.js', (redacted) ] at assertPath (path.js:28:11) at Object.relative (path.js:1252:5) at entryFunctions._.flatMap ((redacted)/packages/functions/node_modules/serverless-webpack/lib/validate.js:154:28) at (redacted)/packages/functions/node_modules/lodash/lodash.js:3563:27 at (redacted)/packages/functions/node_modules/lodash/lodash.js:4925:15 at baseForOwn ((redacted)/packages/functions/node_modules/lodash/lodash.js:3010:24) at (redacted)/packages/functions/node_modules/lodash/lodash.js:4894:18 at baseMap ((redacted)/packages/functions/node_modules/lodash/lodash.js:3562:7) at map ((redacted)/packages/functions/node_modules/lodash/lodash.js:9554:14) at Function.flatMap ((redacted)/packages/functions/node_modules/lodash/lodash.js:9257:26) at ServerlessWebpack.validate ((redacted)/packages/functions/node_modules/serverless-webpack/lib/validate.js:153:31) From previous event: at Object.webpack:validate:validate [as hook] ((redacted)/packages/functions/node_modules/serverless-webpack/index.js:119:10) at BbPromise.reduce ((redacted)/packages/functions/node_modules/serverless/lib/classes/PluginManager.js:372:55) From previous event: at PluginManager.invoke ((redacted)/packages/functions/node_modules/serverless/lib/classes/PluginManager.js:372:22) at PluginManager.spawn ((redacted)/packages/functions/node_modules/serverless/lib/classes/PluginManager.js:390:17) at ServerlessWebpack.BbPromise.bind.then ((redacted)/packages/functions/node_modules/serverless-webpack/index.js:111:51) From previous event: at Object.webpack:webpack [as hook] ((redacted)/packages/functions/node_modules/serverless-webpack/index.js:111:10) at BbPromise.reduce ((redacted)/packages/functions/node_modules/serverless/lib/classes/PluginManager.js:372:55) From previous event: at PluginManager.invoke ((redacted)/packages/functions/node_modules/serverless/lib/classes/PluginManager.js:372:22) at PluginManager.run ((redacted)/packages/functions/node_modules/serverless/lib/classes/PluginManager.js:403:17) at variables.populateService.then ((redacted)/packages/functions/node_modules/serverless/lib/Serverless.js:102:33) at runCallback (timers.js:794:20) at tryOnImmediate (timers.js:752:5) at processImmediate [as _immediateCallback] (timers.js:729:5) From previous event: at Serverless.run ((redacted)/packages/functions/node_modules/serverless/lib/Serverless.js:89:74) at serverless.init.then ((redacted)/packages/functions/node_modules/serverless/bin/serverless:42:50) at ```

Additional Data

HyperBrain commented 6 years ago

Hi @geovanisouza92 , the whole entries mechanism is needed to pickup the right entry again after the compile has been finished and associate the entry name returned from Webpack together with the compile results back to the right serverless function that the compile is for.

I remember that someone copied his custom entries modification into a GitHub issue, prepending the source-map-install and that worked. I do not remember which issue that was but I believe, that he did not add arrays to the entries object, but did something else. I will try to find that again and we can see if that would work here too. If it works, this should be added to the documentation, if it does not work, we have to figure out how to control the entry point name, to be able to use it on the other side of the compile to have the sls function association working. Optimally, this should not break transpiling loaders like TS or Elm.

I'll provide feedback as soon as I've found the comment.

geovanisouza92 commented 6 years ago

Hi @HyperBrain, thanks for the reply!

Well, from the template I generated the project, it already include arrays at entry.


EDIT: What I mean in my suggestion above is that, picking the key will result in the same result as picking the value and manipulating it. For instance:

On my serverless.yml I have a ping function, pointing to src/functions/ping.handler as handler function. This is translated in serverless-webpack to an entry like "src/function/ping": "src/function/ping.ts"; the value (src/function/ping.ts) have its extension (.ts) removed; the allEntryFunctions is generated from serverless handlers, in this case src/function/ping.handler where, again, the "extension" (.handler) is stripped out, remaining src/functions/ping as key to serverless function config.

So, as both places use the same src/functions/ping, maybe we could just use that, ignoring which module(s) compose that entry on webpack itself.

Am I missing some thing?


Indeed, as far as I remember, the compilation output references the entry name (key), so the same src/functions/ping could still be used to match everything.

HyperBrain commented 6 years ago

@geovanisouza92 Thanks for the "edit" šŸ˜„ . The most important point is your very last sentence. As long as the reverse mapping from the Webpack compile stats to the serverless function is unambigous, the behavior should not change. I think doing a prototype in a PR to evaluate if it is feasible or not, would be a good step forward, so that we can decide.

geovanisouza92 commented 6 years ago

So, I started to dig through this implementation and made it work on my fork, but came across a validation that disallow customization like the one I proposed. I didn't noticed this behavior earlier because I was using and older version of serverless-webpack, and since version release v4.1.0 it includes a change described on #272. My suggestion here is that customization should be allowed, but warned, instead throwing a validation error.

In fact, looking at the code I noticed that:


My changes so far include:

Possible changes:

``` functions: foo: handler: foo.handler bar: handler: bar.handler webpack: entry: - ./injected-dependency.js - ./foo.js ```

P.S.: Maybe an alternative is to adjust forceInclude allowing relative paths in addition to module names.

HyperBrain commented 6 years ago

Hi @geovanisouza92 , thanks for the good and detailed proposal šŸŽ‰

As you found correctly, the manual intervention of the entries is blocked since v4. The reason for that is, that for stability everything should be handled by slsw.lib.entries, especially for individual packaging. Turning that into a warning will allow users to break their deployments silently and will decrease the overall plugin stability. So the manual configuration has been deprecated and is not supported anymore.

Additionally, allowing the user to set arbitrary will add much technical debt, because the plugin would have to handle each possible case (which can even change when webpack versions change).

My suggestion here is, to keep the error, but add other fully controlled mechanisms to add arbitrary modules to a function. The whole approach of the configuration in Serverless and its service definition is declarative and so should be the plugin. Your sample at the end would offer the functionality while keeping the declarativeness of the configuration. We should continue to force to set slsw.lib.entries as entries.

What if we just add the possibility to declare additional modules that will be pre- or appended on a function and service level in the serverless.yml. This will solve the issue completely and is easy to understand for users. Any function based config would override the service configuration if any exists. The declaration should never be able to "exchange" the main function module, because that also guarantees the stability of the plugin. An example would be:

# service based config
custom:
  webpack:
    entry:
      pre:
        - source-map-support.js
      post:
        - myAdditionalModule.js
# function override
functions:
  foo:
    handler: foo.handler
  bar:
    handler: bar.handler
    webpack:
      entry:
        pre:
          - ./injected-dependency.js
        post:
          - ./foo.js

All configurations are optional.

IMO this is the only change/extension needed, to have it fully configurable in a deterministic way. What do you think?

geovanisouza92 commented 6 years ago

+1000 to entry prepend/append entries per function.

I was thinking today about using forceInclude to achieve this, but a separate config is better IMO.

I'll work on that later.

geovanisouza92 commented 6 years ago

So, I've updated my fork following this idea of pre/post (actually prepend/append).

Should we support a "global" configuration on custom.webpack.entry? In that case, what should be the precedence?

HyperBrain commented 6 years ago

Yes. As I wrote, the hierarchy is global <- function, i.e. the global config is applied, then the function config if it is present. The effective array contents should then be merged to pertain the order (global then function) As the plugin configuration is abstracted into the Configuration object, it should set empty arrays as defaults (the same as for the forceXXXX configs) and make the entry object available as getter (so that it is read-only).

cryptiklemur commented 6 years ago

ANy progress on this? Or a workaround?

tuanquynet commented 6 years ago

any workaround?

cmbirk commented 5 years ago

How does this operate if the functions are using differing runtimes? Eg. I have one function running node and one running python. I can't supply slsw.lib.entries, because webpack can't compile a python function