Skip to content
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

fix: implemented the multiple webpack compile config #537

Conversation

RishikeshDarandale
Copy link

@RishikeshDarandale RishikeshDarandale commented Sep 5, 2019

What did you implement:

Implemented the fix for providing a way to provide the multiple webpack compile configuration in webpack configuration file. The sample config can be seen as below:

// webpack.config.js

module.exports = [
  {
    entry: './handler1.js',
    target: 'node',
    module: {
      loaders: [ ... ],
    },
  },
  {
    entry: './handler2.js',
    target: 'node',
    module: {
      loaders: [ ... ],
    },
  },
];

Similarly, webpack configuration can export an asynchronous object with multi compile configuration would work as expected.

Closes #439

How did you implement it:

This plugin already supports the multi compile configuration when package.individually is set to true. When there are multiple functions and package.individually is set, then provided single webpack configuration gets cloned for function count times.

Thus, providing multi compile configuration in webpack should be possible with some constraint/assumption.

  • validate function was expecting a single webapck configuration object.
  • Thus, added logic to check if the configuration object is an array or not. If yes, then each config is processed the same way it was done for single configuration object.

When multi webpack compile config is provided, to constraints will be imposed as below:

  • If package.individually is false, then output.path for each config provided should match.
  • If package.individually is true, then compile config should be provided for each function and the output.path of compile should end with function-name.

Above constraints make sure that post validate process such as compile, package, local invoke and offline functions behave as it is.

How can we verify it:

I have verified this with following examples:

  • multi-function with multi compile config provided as shown above.
  • single function with multi compile config provided as shown above.

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

@RishikeshDarandale
Copy link
Author

I will give it a try to resolve the two constraints mentioned above with my solution. Mean time you can provide the feedback for existing changes. Thanks!

@RishikeshDarandale
Copy link
Author

@HyperBrain , Can you please review?

@RishikeshDarandale
Copy link
Author

@HyperBrain , any comment/update on this?

@RishikeshDarandale
Copy link
Author

@HyperBrain, any luck?

@RishikeshDarandale
Copy link
Author

@HyperBrain , Sorry to ping you one more time. Did you get a chance to look into this one?

@miguel-a-calles-mba miguel-a-calles-mba added this to the 5.3.3 milestone May 6, 2020
@miguel-a-calles-mba miguel-a-calles-mba requested a review from a team May 6, 2020 15:03
@miguel-a-calles-mba miguel-a-calles-mba modified the milestones: 5.3.3, 5.4.0 May 6, 2020
@miguel-a-calles-mba miguel-a-calles-mba changed the base branch from master to release/5.4.0 May 6, 2020 23:12
* Add copyExistingArtifacts to packageModules

* Refactor packageModules

* Set service path

* Generate artifact name from service

* Output artifacts to .webpack before copying

* Set artifact name for service packaging

* Skip webpack:compile if --no-build is set

* Add webpack:package:copyExistingArtifacts hook

* Make packageModules & packExternalModules no-op if skipCompile is set

* Refactor packageModules

* Remove artifact location setting from packageModules

* Update cleanup to check this.keepOutputDirectory

* Fix path join on Windows

Co-authored-by: Miguel A. Calles MBA <44813512+miguel-a-calles-mba@users.noreply.github.com>
@tyrauber
Copy link

tyrauber commented May 10, 2020

👍 for this fix. I could use it as well. I've got a webpack.config that builds several different entries, with different outputs and plugins, and serverless with serverless-webpack chokes on module.exports = [server, web, serverless] with the following error:

  Error: Plugin could not be registered at 'before-compile'. Hook was not found.
  BREAKING CHANGE: There need to exist a hook at 'this.hooks'. To create a compatibility layer for this hook, hook into 'this._pluginCompat'.

Edit: Workaround, totally silly, but works.

If you have a webpack.config.js, with multiple exports as above, you can create a secondary webpack config, say webpack.serverless.js, that compiles the previous exports and returns the last like so:

const Webpack = require('webpack')
const config = require('./webpack.config.js')
const webCompiler = Webpack(config.shift(), (err, stats) => {})
const serverCompiler = Webpack(config.shift(), (err, stats) => {})
module.exports = config.pop()

Then in your serverless.yml:

 webpack:
    webpackConfig: ./webpack.serverless.js

This will allow you to group various entries with unique outputs and plugins in a single config, and run them (or some subset) prior to running the serverless webpack entry.

@RishikeshDarandale
Copy link
Author

@tyrauber , if you have your code somewhere on github, then I will take a look. If not passible to share the code, then share the snippet.

@tyrauber
Copy link

Hi @RishikeshDarandale, here is a gist of a Serverless Vue Webpack 4 Config that details the issue. The problem comes from wanting to code split and share a base configuration and use webpack-merge to maintain them. Each entry - client, server, serverless - potentially needs different plugins or outputs. Unfortunately, serverless-webpack chokes on an array of exports, while webpack handles it fine.

miguel-a-calles-mba and others added 3 commits May 15, 2020 15:39
- multi compile config option implemented with checking the config object is an
array or not.
- when package.individually is false, then all config should have same exact
output.path of each compile config.
- when package.individually is true, compile config should be equal to or
greater than function count and output.path of each config of a function should
ends with functon name

With above imlementation, the rest did not need to change and work as it is.
@RishikeshDarandale RishikeshDarandale force-pushed the topics/multiple-file-configuration branch from d6a6cb5 to a500a84 Compare May 23, 2020 10:18
@RishikeshDarandale
Copy link
Author

@miguel-a-calles-mba Can you please review this wrt master or release/5.3.2?

Please see here for more comments on #560 .

@RishikeshDarandale
Copy link
Author

@miguel-a-calles-mba , any update on this pull request?

@miguel-a-calles-mba
Copy link
Member

@RishikeshDarandale This PR is set for 5.4.0.

@RishikeshDarandale
Copy link
Author

sure @miguel-a-calles-mba

I have rebased my changes with release 5.4.0locally, but 5.4.0 has an package issue. This has to wait till it gets fixed or if you say that it's not an issue, then I can push my rebased changes!

@miguel-a-calles-mba
Copy link
Member

@RishikeshDarandale, I resolved the conflicts. Please verify the changes.

@j0k3r j0k3r closed this Jan 29, 2021
@j0k3r j0k3r deleted the branch serverless-heaven:master January 29, 2021 14:25
@j0k3r j0k3r reopened this Jan 29, 2021
@j0k3r j0k3r changed the base branch from release/5.4.0 to master January 29, 2021 19:16
@j0k3r
Copy link
Member

j0k3r commented Jan 29, 2021

Sorry I made a mistake while merging release/5.4.0 into master and removed it. The removal automatically closed that PR. It should now be rebased against the master and fix conflicts.

@j0k3r j0k3r self-assigned this Jan 29, 2021
@j0k3r j0k3r removed this from the 5.4.0 milestone Mar 4, 2021
Copy link
Member

@miguel-a-calles-mba miguel-a-calles-mba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address the merge conflicts and the failed builds. Thanks.

@j0k3r
Copy link
Member

j0k3r commented May 10, 2021

@RishikeshDarandale any chances to rebase against the master, fix conflicts and tests? Thanks 🙏

@j0k3r j0k3r added the awaiting reply Awaiting for a reply from the OP label May 10, 2021
@RishikeshDarandale
Copy link
Author

@j0k3r , let me find some time to check this one. It's a year old change now.

@j0k3r
Copy link
Member

j0k3r commented Nov 21, 2021

Will you have time to update it?

@j0k3r j0k3r closed this Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting reply Awaiting for a reply from the OP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Errors when using a Webpack config with multiple configurations
5 participants