Individually packaging with multi-modules entry does not work

[geovanisouza92]

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:

• entry receives the relative filename of each handler
• entryFile receives entry without extension
• entryFile is used to pick the right entry function from allEntryFunctions
• allEntryFunctions is generated from the entries, where handlerFile is generated from function handlers on serverless.yml.

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:

• What went wrong?

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) ]"

• What did you expect should have happened?

The commands sls invoke local and sls webpack working fine.

• What was the config you used?

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" }],
  },
};

• What stacktrace or error message from your provider did you see?

  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 <anonymous>

Additional Data

• Serverless-Webpack Version you're using: 4.0.0
• Webpack version you're using: 3.6.0
• Serverless Framework Version you're using: 1.27.2
• Operating System: Windows Subsystem for Linux on Windows 10
• Stack Trace (if available): (above)

[HyperBrain]

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]

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]

@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]

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:

• It provides an automatic discovery of corresponding modules for each function handler;
• It requires the webpack config, allowing a backward-compatible usage of slsw.lib.entries (which misses the point, if customization is disallowed);
• In the code that handles multi-compilation, it refers to (auto-discovered) entries, which could possibly be changed by webpack.config.js, as my use case described above.

---

My changes so far include:

• Warn instead error on entry customization;
• "Sync" lib.entries after webpackConfig was required;
• Don't assume that it's only one module per entry, but use the entries map to resolve each handler func to corresponding modules;
• No major breaking changes (please, review to ensure).

Possible changes:

• Add an option on webpack section (per function) allowing entry manipulation without writing a webpack.config.js. Example:

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]

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]

+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]

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]

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]

ANy progress on this? Or a workaround?

[tuanquynet]

any workaround?