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

Core - Simplify custom webpack config #4927

Merged
merged 5 commits into from
Dec 21, 2018
Merged

Conversation

igor-dv
Copy link
Member

@igor-dv igor-dv commented Dec 5, 2018

Issue: Partially related to #4903

Today we have an option to extend webpack config by providing the function like this:

module.exports = (basicConfig, mode, defaultConfig) => { /*...*/ }

Where defaultConfig is a few rules bigger than a basicConfig. I think it brings more confusion rather a benefit.

What I did

  1. Reduced the basicConfig and changed to the signature to:
module.exports = ({ config, mode }) => { /*...*/ }

Why object? Because it's much easier to extend and deprecate in the future.

  1. Removed a minor prop (defaultConfigName) that is passed forme the frameworks to core. This prop is kinda not needed.

@codecov
Copy link

codecov bot commented Dec 5, 2018

Codecov Report

Merging #4927 into next will increase coverage by 0.02%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##             next    #4927      +/-   ##
==========================================
+ Coverage   35.02%   35.04%   +0.02%     
==========================================
  Files         566      566              
  Lines        7004     7000       -4     
  Branches      937      936       -1     
==========================================
  Hits         2453     2453              
+ Misses       4055     4052       -3     
+ Partials      496      495       -1
Impacted Files Coverage Δ
app/ember/src/server/options.js 0% <ø> (ø) ⬆️
app/react/src/server/options.js 0% <ø> (ø) ⬆️
app/angular/src/server/options.js 0% <ø> (ø) ⬆️
...b/core/src/server/preview/custom-webpack-preset.js 0% <0%> (ø) ⬆️

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 3a06253...76ae1d4. Read the comment docs.

@igor-dv igor-dv added this to the v5.0.0 milestone Dec 5, 2018
@storybook-safe-bot
Copy link
Contributor

Fails
🚫 PR is marked with "do not merge", "BREAKING CHANGE" labels.

Generated by 🚫 dangerJS

@ndelangen
Copy link
Member

Shall we merge this into the overhaul-ui branch?

@ndelangen ndelangen changed the base branch from next to 5.0.0 December 21, 2018 18:57
@ndelangen ndelangen merged commit 55db5a3 into 5.0.0 Dec 21, 2018
@ndelangen ndelangen deleted the core/remove-basic-config branch December 21, 2018 19:37
@Hypnosphi
Copy link
Member

Hypnosphi commented Jan 17, 2019

What is the recommended way of getting only the absolutely required part of default config now?

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

Successfully merging this pull request may close these issues.

5 participants