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

reduce coupling between webpack and webpack-cli #229

Closed
wants to merge 1 commit into from

Conversation

sokra
Copy link
Member

@sokra sokra commented Dec 30, 2017

What kind of change does this PR introduce?
refactoring

Did you add tests for your changes?
existing tests

If relevant, did you update the documentation?
N/A

Summary

  • reduce coupling between webpack and webpack-cli
  • require("webpack/lib/DefinePlugin") -> require("webpack").DefinePlugin
  • move two files from webpack to webpack-cli. They are only used by webpack-cli.

Does this PR introduce a breaking change?

yes, since webpack.Stats is not exposed by webpack versions <= 4.0.0-alpha.2, this change will only work with higher versions. It was exposed by webpack/webpack@289ba67

Other information
The goal is to reduce the coupling between webpack and webpack-cli.
webpack-cli should only use the public webpack API.

@sokra
Copy link
Member Author

sokra commented Dec 30, 2017

This can only be merged when the next webpack alpha/beta is released.

@sokra
Copy link
Member Author

sokra commented Jan 7, 2018

latest alpha has been published, but PR is now conflicting...

@evenstensberg
Copy link
Member

I'll check it out later and fix the conflict

@evenstensberg
Copy link
Member

@sokra Rebased and reinstalled webpack ( webpack@alpha.4.0.0-alpha.2, but test suits are failing. Is it related to @TheLarkInn 's missing tests on 0CJS? If so, then #238 should be reviewed and merged first, then we'll do a rebase.

Also, when rebasing, the entire bin/webpack.js got a whole lot different than from master and your branch. May be better to open a new PR if it's comparing old hashes instead of the latest ones

@evenstensberg
Copy link
Member

Following up on this, this week

Try to use only the public webpack API
@evenstensberg
Copy link
Member

I tried to rebase the execHelpers change, but realized that it has been implemented in #251 . Closing in favor

@evenstensberg evenstensberg deleted the cleanup/remove-webpack-deps branch February 14, 2018 00:12
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.

3 participants