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.

Sign up for GitHub

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

Jump to bottom

WIP: Webpack v4 #354

Closed
wants to merge 14 commits into from
Closed

WIP: Webpack v4 #354

wants to merge 14 commits into from

Conversation

dmbch
Copy link
Contributor

@dmbch dmbch commented Jan 26, 2018 •
edited
Loading

In this PR and branch, we are preparing the forthcoming upgrade to Webpack v4.

  • upgrade webpack to v4
  • update configs to basically work
  • re-enable extract-text-webpack-plugin
  • review and thoroughly revise configs
  • fix chunking (CommonsChunkPlugin is deprecated)
  • get rid of manifest.js mechanism (config, react, template...)
  • investigate sideEffects: false

@dmbch dmbch self-assigned this Jan 26, 2018
@dmbch dmbch mentioned this pull request Jan 29, 2018
Closed
3 tasks
@dmbch dmbch force-pushed the webpack4 branch 5 times, most recently from c51e85a to 38dd6ab Compare February 2, 2018 21:12
@dmbch
Copy link
Contributor Author

dmbch commented Feb 2, 2018 •
edited
Loading

Re sideEffects: false: this is an ESM-only feature - we ought to enable this as soon as hops-{react, redux,graphql} are migrated to ESNext.

@dmbch dmbch force-pushed the webpack4 branch 4 times, most recently from 86678f1 to 25b28c6 Compare February 5, 2018 16:41
@ZauberNerd
Copy link
Contributor

Waiting for one of the following issues/PRs to be resolved:

We could also investigate switching to extract-css-chunks-webpack-plugin once this PR is merged:

@dmbch
Copy link
Contributor Author

dmbch commented Feb 14, 2018 •
edited
Loading

@ZauberNerd since we discussed making this upgrade non-breaking, I checked what we broke, here. As far as I can tell, these are the two breaking changes introduced in this PR. We could re-introduce startServer (paging @robin-drexler), but honestly, replacing it downstream should be quite simple.

The first one, however, is the bigger issue. Reverting/mitigating that one would essentially make us re-implement WDS...

56315a3: refactor(build): get rid of WDS, use Express

BREAKING CHANGE: Since we got rid of webpack dev server, its config is
no longer supported. All existing hops features (including SSL) are
still supported, though.

d11c9d5: refactor(express): convert exports to getters

BREAKING CHANGE: remove legacy startServer export

What do you think?

ZauberNerd
ZauberNerd approved these changes Feb 14, 2018
View reviewed changes
packages/build-config/index.js Outdated
@@ -2,7 +2,16 @@

var hopsConfig = require('hops-config');

module.exports = Object.freeze({
module.exports = exports = {
get build() {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

packages/build-config/loaders/config.js Outdated
@@ -20,7 +20,7 @@ function getConfig(config) {
.reduce(function(result, key) {
result[key] = (function shorten(item) {
if (typeof item === 'string') {
return item.replace(new RegExp(config.appDir), '.');
return path.relative(config.appDir, item);
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

@ZauberNerd
Copy link
Contributor

LGTM, Although I'd prefer to split this into fixes / features / refactoring that we can deploy with the current Hops@10 and the breaking changes / webpack 4 integration that we'll release as a prerelease.

@dmbch
Copy link
Contributor Author

dmbch commented Feb 14, 2018

@ZauberNerd agreed. Could you help with that?

And how do you think we should deal with the DevServer stuff? If we treat it as non-breaking, we could merge it before the Webpack stuff...

@ZauberNerd ZauberNerd force-pushed the webpack4 branch 10 times, most recently from 2bd05a6 to a7eb2e4 Compare February 20, 2018 21:13
ZauberNerd and others added 14 commits February 27, 2018 13:21
@ZauberNerd
chore: reformat all package.json files with prettier
1e98b6d
@dmbch @ZauberNerd
feat(build): update webpack to v4
33b47d5
@dmbch @ZauberNerd
feat(build-config): update webpack to v4
416aecf 
fix configs, temporarily remove extract-text-webpack-plugin
@dmbch @ZauberNerd
refactor(build-config): get rid of manifest.js mechanism
ffb0155
@dmbch @ZauberNerd
refactor(react): get rid of manifest.js mechanism
4163dde
@dmbch @ZauberNerd
test(spec): remove manifest checks, fix CSS checks
556af97 
temporarily disable CSS checks while waiting for ExtractTextPlugin to update
@dmbch @ZauberNerd
refactor(react): use assets info provided using express
1412eeb 
stop relying on custom loader, use the pattern introduced by webpack dev middleware
@dmbch @ZauberNerd
feat(build-config): get rid of manifest plugin, simplify config loader
c0f482a 
both are superseded by new express based asset handling
@dmbch @ZauberNerd
feat(build-config): tweak Webpack configs for v4
a46a28d
@dmbch @ZauberNerd
feat(build-config): introduce global css mode
bc3a94a 
By importing css files using a query string, css modules can now be deactivated.

```js
import './foo.css?global';      // will leave class names unchanged
import styles from './bar.css'; // will rewrite class names
```
@dmbch @ZauberNerd
docs(config): get rid of obsolete info on assets, manifest fields
11687f5
@dmbch @ZauberNerd
refactor(build-config): get rid of (now obsolete) uglify plugin
8016ffe
@dmbch @ZauberNerd
feat(build-config): update to latest alpha of extract text plugin
b5568e3
@dmbch @ZauberNerd
fix(spec): re-enable CSS build test
49732e8
@dmbch dmbch mentioned this pull request Feb 27, 2018
Closed
3 tasks
@dmbch
Copy link
Contributor Author

dmbch commented Mar 12, 2018

@ZauberNerd should we not work on #417 in lieu of this PR, i.e. close this..?

@ZauberNerd ZauberNerd closed this Mar 12, 2018
@ZauberNerd ZauberNerd deleted the webpack4 branch March 12, 2018 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ZauberNerd ZauberNerd ZauberNerd approved these changes

@robin-drexler robin-drexler Awaiting requested review from robin-drexler

@matthias-reis matthias-reis Awaiting requested review from matthias-reis

Assignees

@dmbch dmbch

Labels
🔨 build 🎛 config 🔝 enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dmbch @ZauberNerd