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

refactor(webpack): add ability to get webpack config class #5378

Merged
merged 5 commits into from
Mar 31, 2019

Conversation

damianstasik
Copy link

@damianstasik damianstasik commented Mar 25, 2019

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

This PR is an improvement of #5366 and adds a new method to WebpackBundler that returns a webpack config class (WebpackClientConfig, WebpackServerConfig or WebpackModernConfig).

I also cleaned up the build method a bit and moved the plugin aliases into WebpackClientConfig and WebpackServerConfig so that each class has all of it's webpack aliases in one place.

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly. (PR: #)
  • I have added tests to cover my changes (if not applicable, please state why)
  • All new and existing tests are passing.

@codecov-io
Copy link

codecov-io commented Mar 25, 2019

Codecov Report

Merging #5378 into dev will decrease coverage by 0.03%.
The diff coverage is 95%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #5378      +/-   ##
==========================================
- Coverage   96.01%   95.97%   -0.04%     
==========================================
  Files          74       74              
  Lines        2532     2535       +3     
  Branches      641      640       -1     
==========================================
+ Hits         2431     2433       +2     
- Misses         85       86       +1     
  Partials       16       16
Impacted Files Coverage Δ
packages/webpack/src/config/client.js 98.18% <100%> (+0.18%) ⬆️
packages/webpack/src/config/server.js 100% <100%> (ø) ⬆️
packages/webpack/src/builder.js 93.54% <90%> (-1.46%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3b85dd9...d676ac8. Read the comment docs.

@galvez
Copy link

galvez commented Mar 26, 2019

@visualfanatic Please ensure the test suite passes first.

@damianstasik
Copy link
Author

@galvez @pi0 fixed, could you take a look?

@atinux atinux requested a review from clarkdo March 28, 2019 11:07
@@ -37,44 +37,32 @@ export class WebpackBundler {
}
}

getWebpackConfig(name) {
Copy link
Member

@clarkdo clarkdo Mar 28, 2019

Choose a reason for hiding this comment

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

How about returning config object here, then we don't need an extract bind.

Otherwise, if we call config separately, we can leverage Promise.all to enhance config() performance.

Copy link
Member

@pi0 pi0 Mar 31, 2019

Choose a reason for hiding this comment

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

@clarkdo Can you please elaborate more about bind?

This function is not async to leverage Promise.all. The runner will be specified based on build type here

Copy link
Member

Choose a reason for hiding this comment

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

@clarkdo Refactor applied. Please review again.

Copy link
Author

Choose a reason for hiding this comment

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

@pi0 your change was a surprise, but it was appreciated, thank you.

@clarkdo clarkdo changed the title feat(webpack): add ability to get webpack config class refactor(webpack): add ability to get webpack config class Mar 28, 2019
@clarkdo
Copy link
Member

clarkdo commented Mar 28, 2019

Changed title to refactor for being eligible for v2.5.2

@clarkdo clarkdo merged commit abf7db1 into nuxt:dev Mar 31, 2019
@pi0 pi0 mentioned this pull request Mar 31, 2019
@juliendargelos
Copy link

As I proposed here, I think it could be useful for some advanced modules to be able to extend the webpack base config class for independent webpack build based on Nuxt base config. It's just about exposing WebpackBaseConfig (and possibly WebpackClientConfig, WebpackServerConfig and WebpackModernConfig) in index.js:

export { WebpackBundler as BundleBuilder } from './builder'
export { default as WebpackBaseConfig } from './config/base'

@pi0
Copy link
Member

pi0 commented Apr 3, 2019

There is no guarantee about structure of webpack config classes to be the same across releases. So one can use final result and use/extend it for advanced use cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants