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

feat: sharabale configs #2868

Closed
wants to merge 5 commits into from
Closed

feat: sharabale configs #2868

wants to merge 5 commits into from

Conversation

snitin315
Copy link
Member

What kind of change does this PR introduce?
FEATURE

Did you add tests for your changes?
yes, more tests WIP
If relevant, did you update the documentation?
WIP
Summary
Fixes #2748

Does this PR introduce a breaking change?
No

Other information

WIP in covering more edge cases with multiple nested configs and multi compiler mode.

@snitin315 snitin315 requested a review from a team as a code owner August 2, 2021 10:29
@snitin315 snitin315 marked this pull request as draft August 2, 2021 10:29
@codecov
Copy link

codecov bot commented Aug 2, 2021

Codecov Report

Merging #2868 (8e352fa) into master (ce2c68d) will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2868      +/-   ##
==========================================
+ Coverage   94.95%   94.99%   +0.04%     
==========================================
  Files          31       31              
  Lines        1704     1719      +15     
  Branches      484      488       +4     
==========================================
+ Hits         1618     1633      +15     
  Misses         86       86              
Impacted Files Coverage Δ
packages/webpack-cli/lib/webpack-cli.js 96.81% <100.00%> (+0.04%) ⬆️

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 ce2c68d...8e352fa. Read the comment docs.

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Let's add tests with --merge too

@snitin315
Copy link
Member Author

I am also implementing --extends flag with this.

@alexander-akait
Copy link
Member

Yep, it is great idea

@snitin315 snitin315 force-pushed the feat/sharable-configs branch from 506f46b to 4f5de57 Compare August 7, 2021 13:35
@snitin315 snitin315 changed the title [WIP] feat: sharabale configs feat: sharabale configs Aug 9, 2021
@snitin315 snitin315 marked this pull request as ready for review August 9, 2021 12:26

await this.resolveConfig(newOptions);
}
});
Copy link
Member

Choose a reason for hiding this comment

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

We do not use webpack-merge logic here? it is working good

Copy link
Member Author

Choose a reason for hiding this comment

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

We will also need to load the extended configurations, here we have only paths for extended configurations and not their options.

Or am I missing something here?

Copy link
Member

Choose a reason for hiding this comment

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

Ideally --extends means --config=webpack.config.js --config=webpack.other.config.js --merge, so I think we already have logic for this, for the next major release we will change logic for multiple --config https://github.com/webpack/webpack-cli/blob/master/packages/webpack-cli/lib/webpack-cli.js#L1619, I think we will throw an error on two and more config options without --merge, but we can reuse this logic for --extends, --extends like syntax sugar, also do not forget --extends in CLI should override extends on webpack.config.js (I think eslint works the same, but need check)

Copy link
Member Author

Choose a reason for hiding this comment

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

I will refactor 👍🏻 .

@alexander-akait
Copy link
Member

Fixed #3738

@alexander-akait alexander-akait deleted the feat/sharable-configs branch April 29, 2023 17:22
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.

Shareable Configs
3 participants