version 1.0 #742

alexander-akait · 2018-07-04T21:46:21Z

What kind of change does this PR introduce?

refactor

Did you add tests for your changes?

already exists

If relevant, did you update the README?

yes

Summary

Roadmap:

Merge this PR and release 1.0.0 version.
Drop css modules in favor https://github.com/css-modules/postcss-modules
Refactor code (get some usefull stuff from [WIP] refactor(loader): v2.0.0 #639) and webpack-default
Release 2.0.0 version.

Does this PR introduce a breaking change?

yes

Other information

Let's do it

fixes #691 fixes #265 fixes #688 fixes #732

# Conflicts: # test/simpleTest.js

BREAKING CHANGE: remove `minimize` option

BREAKING CHANGE: remove `module` option, use `modules` option instead

BREAKING CHANGE: remove `camelcase` option, use `camelCase` option instead

BREAKING CHANGE: remove `root` option, use `postcss-loader` with `postcss-url` plugin instead

BREAKING CHANGE: remove `alias` option, use `webpack` `resolve.alias` option

BREAKING CHANGE: minimum require `nodejs` version is `6.9`

alexander-akait · 2018-07-04T21:46:51Z

/cc @ai

Can you help me with better integration postcss-loader with postcss-modules?

ai · 2018-07-04T22:33:46Z

Sure. What I need to look?

alexander-akait · 2018-07-04T22:37:22Z

@ai need find way how use postcss-loader with postcss-modules plugin (our pipeline https://github.com/webpack-contrib/css-loader/blob/master/lib/processCss.js#L189), maybe some values will be unavailable inside postcss-loader.

In short: need add example postcss-modules configuration with postcss-loader in README.

ai · 2018-07-04T22:41:30Z

👍 will send you a PR in next few days

shellscape

A few notes. Should webpack-defaults be merged into this repo before or after this PR is merged?

shellscape · 2018-07-05T02:17:53Z

.travis.yml

@@ -7,20 +7,20 @@ matrix:
  fast_finish: true
  include:
    - os: linux
-      node_js: "7"
-      env: WEBPACK_VERSION="2.2.0" JOB_PART=lint
+      node_js: "10"


can we ditch travis and get on circle CI?

@shellscape let's do this in other PR

shellscape · 2018-07-05T02:18:34Z

lib/loader.js

-	var root = query.root;
-	var moduleMode = query.modules || query.module;
-	var camelCaseKeys = query.camelCase || query.camelcase;
+	var moduleMode = query.modules;


because this is a refactor, shouldn't we use const and let instead?

@shellscape also let's do it other PR, when we ship webpack-default in this repo

shellscape · 2018-07-05T02:19:47Z

package.json

@@ -36,6 +34,9 @@
    "should": "^11.1.2",
    "standard-version": "^4.0.0"
  },
+  "peerDependencies": {
+    "webpack": "^3.0.0 || ^4.0.0"


new major versions of webpack-contrib and webpack org modules are only supporting webpack@4 moving forward. since this is a new major version, we should drop webpack@3 support.

Agree with Andrew, it's harder to keep compat when things move forward too

@shellscape 👍

alexander-akait · 2018-07-05T09:42:28Z

@shellscape i think we can update webpack-default after merge this PR, because problem with browserslist are ASAP

BREAKING CHANGE: minimum require `webpack` version is `4`

codecov · 2018-07-05T09:44:27Z

Codecov Report

Merging #742 into master will decrease coverage by 0.18%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #742      +/-   ##
==========================================
- Coverage   98.43%   98.25%   -0.19%     
==========================================
  Files          11       10       -1     
  Lines         384      344      -40     
  Branches       90       76      -14     
==========================================
- Hits          378      338      -40     
  Misses          6        6

Impacted Files	Coverage Δ
lib/loader.js	`100% <100%> (ø)`	⬆️
lib/localsLoader.js	`100% <100%> (ø)`	⬆️
lib/processCss.js	`98.54% <100%> (-0.13%)`	⬇️

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 1703721...8db3fb0. Read the comment docs.

sokra · 2018-07-05T15:40:02Z

There are few more breaking changes possible, but that should be considered carefully:

Dropping `~` requests, urls -> requests

Currently it behaves like this:

url("./something") -> require("./something")
url("something") -> require("./something")
url("~something") -> require("something")

We can consider dropping this logic:

url("./something") -> require("./something")
url("something") -> require("something")

The logic was there for best compatiblity with existing stylesheets, but this may not be thing for everybody.

As workaround one can do this:

module.rules: [
  { test: /\.css$/,
    use: [MCEP.loader, "css-loader"],
    resolve: { modules: [".", "node_modules"] }
  }
]

which will change the logic to this:

url("./something") -> require("./something")
url("something") -> require("something") but looks in the current directory too.

For backward-compat ~ could be supported but deprecated. Or a separate loader can do the transform for backward-compat.

Dropping `importLoaders`

Currently when using @import (and it's done by the css-loader, which is not the case when using sass or postcss-import) X loaders are copied to the inner require. X = importLoaders.

This is important to not apply the style-loader to inner requests, because that doesn't work.

When dropping importLoaders you have to manually make sure that style-loader is not appied to inner requests, i. e. with these rules:

module.rules: [
 { test: /\.css$, rules: [
    { issuer: { not: /\.css$/ }, use: "style-loader" },
    { use: "css-loader" }
 ]}
]

Similar rule for MCEP.

ESM exports

When using "locals" (modules) the locals could be exported as ESM exports. This would make the result a ESM. For backward-compat expose it as named exports and all exports as object via default export.

ooflorent · 2018-07-06T07:12:26Z

package.json

@@ -36,6 +34,9 @@
    "should": "^11.1.2",
    "standard-version": "^4.0.0"
  },
+  "peerDependencies": {
+    "webpack": "^4.0.0"


Not sure about this. The codebase does not require('webpack'). I understand that you added this to avoid runtime errors when webpack's version is not the expected one but I don't think this is the correct solution.

@ooflorent we use this for all webpack-contrib repos, better create issue in webpack-default about this.

@ooflorent Also it is prevent problem with using webpack api between version, if somebody want use css-loader on webpack@3, they can use old version

yeah it's a standard for webpack-defaults. that peerDep on webpack has been in defaults for a very long time. along with the Requirements section in the defaults README, this serves as a warning to keep people on supported versions of webpack. new major versions of loaders/plugins/utils are webpack@4+ and Node 6+ only.

alexander-akait · 2018-07-06T16:27:35Z

Let's do it and do css-loader better in future

evilebottnawi and others added 13 commits May 2, 2018 16:05

tests: css custom variables (#709)

07444ad

Merge remote-tracking branch 'origin/master'

50f8ec0

# Conflicts: # test/simpleTest.js

Merge remote-tracking branch 'origin/master'

1b7acf7

refactor: remove minimize option

965068d

BREAKING CHANGE: remove `minimize` option

refactor: remove module option

b47c566

BREAKING CHANGE: remove `module` option, use `modules` option instead

refactor: remove camelcase option

871e905

BREAKING CHANGE: remove `camelcase` option, use `camelCase` option instead

refactor: remove root option

594a4c2

BREAKING CHANGE: remove `root` option, use `postcss-loader` with `postcss-url` plugin instead

refactor: remove other minimizer stuff

8f9ea06

refactor: remove alias option

1cb903b

BREAKING CHANGE: remove `alias` option, use `webpack` `resolve.alias` option

refactor: readme

9907f13

refactor: remove unused object-assign

88c38e5

chore(deps): update postcss to latest version

c5b2cc1

refactor: minimum require nodejs version is 6.9

4fa385c

BREAKING CHANGE: minimum require `nodejs` version is `6.9`

alexander-akait changed the title ~~1.0~~ version 1.0 Jul 4, 2018

alexander-akait requested review from shellscape and ooflorent July 4, 2018 21:47

alexander-akait force-pushed the 1.0 branch from f4b962a to 9cce802 Compare July 4, 2018 21:54

alexander-akait added this to the 1.0.0 milestone Jul 4, 2018

shellscape suggested changes Jul 5, 2018

View reviewed changes

refactor: minimum require webpack version is 4

8db3fb0

BREAKING CHANGE: minimum require `webpack` version is `4`

alexander-akait force-pushed the 1.0 branch from 9cce802 to 8db3fb0 Compare July 5, 2018 09:44

shellscape approved these changes Jul 5, 2018

View reviewed changes

ooflorent reviewed Jul 6, 2018

View reviewed changes

alexander-akait mentioned this pull request Jul 6, 2018

Remove minimize option #741

Closed

This was referenced Jul 6, 2018

chore(package): drop Node.js 4 support #740

Closed

chore(package): use cssnano 4 and PostCSS 6 #739

Closed

alexander-akait merged commit 240db53 into master Jul 6, 2018

alexander-akait deleted the 1.0 branch July 6, 2018 16:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

version 1.0 #742

version 1.0 #742

alexander-akait commented Jul 4, 2018 •

edited

Loading

alexander-akait commented Jul 4, 2018 •

edited

Loading

ai commented Jul 4, 2018

alexander-akait commented Jul 4, 2018

ai commented Jul 4, 2018

shellscape left a comment

shellscape Jul 5, 2018

alexander-akait Jul 5, 2018

shellscape Jul 5, 2018

shellscape Jul 5, 2018

alexander-akait Jul 5, 2018

shellscape Jul 5, 2018

shellscape Jul 5, 2018

evenstensberg Jul 5, 2018

alexander-akait Jul 5, 2018

alexander-akait commented Jul 5, 2018

codecov bot commented Jul 5, 2018 •

edited

Loading

sokra commented Jul 5, 2018

ooflorent Jul 6, 2018

alexander-akait Jul 6, 2018

alexander-akait Jul 6, 2018

shellscape Jul 6, 2018

alexander-akait commented Jul 6, 2018

version 1.0 #742

version 1.0 #742

Conversation

alexander-akait commented Jul 4, 2018 • edited Loading

alexander-akait commented Jul 4, 2018 • edited Loading

ai commented Jul 4, 2018

alexander-akait commented Jul 4, 2018

ai commented Jul 4, 2018

shellscape left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexander-akait commented Jul 5, 2018

codecov bot commented Jul 5, 2018 • edited Loading

Codecov Report

sokra commented Jul 5, 2018

Dropping ~ requests, urls -> requests

Dropping importLoaders

ESM exports

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexander-akait commented Jul 6, 2018

alexander-akait commented Jul 4, 2018 •

edited

Loading

alexander-akait commented Jul 4, 2018 •

edited

Loading

codecov bot commented Jul 5, 2018 •

edited

Loading

Dropping `~` requests, urls -> requests

Dropping `importLoaders`