Initial ES6 modules support #402
Conversation
|
|
lib/compile-exports.js
Outdated
|
|
||
| return res; | ||
| }, []).join(",\n"); | ||
| }, []); | ||
| exportJs.push("export default { " + symbols.join(", ") + " };"); |
There was a problem hiding this comment.
It should not default export the key, but export each symbol separate. Best emit an
export var className = "class-name";per class name.
The CSS itself should be exported via default export.
export default css;There was a problem hiding this comment.
This means class name always need to be converted to camelCase from dashed-case
There was a problem hiding this comment.
It does both, exports each separate symbol (line 28) but also exports all symbols as properties on one default object (line 33).
This was done for compatibility with the CSS modules spec where eg import styles from './styles.css'; is expected to give you such an object. Exporting the CSS source by default makes more sense in terms of a loader but wouldn't allow that behaviour. It also allows dashed-case inside the default object, although not in the separated symbols.
lib/loader.js
Outdated
|
|
||
| var js = "import { $cssLoader as $cssLoader" + hash + " } from '" + importUrl + "';\n"; | ||
| if (symbols.length) { | ||
| js = js + "import { " + symbols.join(", ") + " } from '" + importUrl + "';\n"; |
There was a problem hiding this comment.
Instead of '" + importUrl + "' always use JSON.stringify(importUrl), elsewise if breaks for chars that need to be escaped.
.babelrc
Outdated
| @@ -0,0 +1,3 @@ | |||
| { | |||
| "presets": ["es2015"] | |||
There was a problem hiding this comment.
Is this necessary? You specify the preset in the tests specifically.
I've tried to address this by adding a toString function which returns the CSS itself. Hopefully this is an acceptable solution as the CSS is available but it also works the way CSS Modules expect. Another tradeoff is skipping separate export of classnames which clash with JS reserved words (eg .class), but these are still exported in the default object. |
Codecov Report
@@ Coverage Diff @@
## master #402 +/- ##
=========================================
+ Coverage 98.4% 98.9% +0.49%
=========================================
Files 9 11 +2
Lines 314 365 +51
Branches 71 78 +7
=========================================
+ Hits 309 361 +52
+ Misses 5 4 -1
Continue to review full report at Codecov.
|
|
@simlrh - Can you pick up master when you have a second, it has a bunch of applicable Travis updates. |
|
Regarding using tree shaking to remove unused rules from the CSS, I've implemented this as a plugin called webpack-dead-css, demo project here. |
|
Changed importLoader behaviour to allow loaders to run before and after css-loader on imported modules. Necessary so that babel-loader can be applied to the ES6 output. Eg: {
loader: 'css-loader',
query: {
importLoaders: {
beforeCssLoader: [ 'postcss-loader' ],
afterCssLoader: [{
loader: 'babel-loader',
query: {
presets: [ ['es2015', {modules: false}] ]
}
}]
}
}
} |
|
Not sure now that changing importLoader behaviour is the best solution. If loaders are going to return ES2015 modules then babel-loader will need to be added to the top of the loader chain for every file type, as well as importLoaders on any imported CSS. (I had 3 babel-loader invocations in the demo project for this PR). And webpack 2 can handle ES2015 anyway, so there's no actual need to transpile in a loader any more. I think the better solution is to forget babel-loader and run babel during "optimize-chunk-assets" like UglifyJsPlugin. See babel-webpack-plugin for this behaviour. |
|
FYI the CLA check has been broken since the repo moved organisation so I can't access it to sign again, but I did previously sign the agreement: #402 (comment) |
|
@simlrh This PR is definitely stalled which is shame tbh, contains a lot of the fixes we recently merged or working on. If you still have the motivation to rebase this PR and refactor all the superseded parts out feel free to do so and let my know :)
Otherwise I will close this PR and for still outstanding issues that were adressed by this PR feel free to send PR after the |
|
Closing for now but feel free to reopen :) |
What kind of change does this PR introduce?
Imports and exports changed to use the native ES6 module system.
importLoaders option changed to allow importLoaders to run after the css-loader (to allow babel to run on imported css modules).
Did you add tests for your changes?
Yes, updated tests.
If relevant, did you update the README?
Yes
Summary
ES6 exports work with tree shaking, which makes it possible to track which CSS classes are used and remove dead CSS (ie in a plugin that fires prior to minification).
Does this PR introduce a breaking change?
This version requires a new style-loader to consume the exports correctly, and the same for any loader expecting output from css-loader. A PR for a compatible style-loader is being posted along with this one.
Other information
"@import" statements import the default object from imported css. When using "composes:" only the symbols used are imported.
CSS classes are exported separately as camel case variables. An object containing all classes is exported as the default. Classes that clash with JS reserved words are not exported separately but are included in the default. The default object has a toString function which returns the CSS itself.
In addition an object called $css is exported which contains the css source & sourcemap and a list of imported css media query information to help style-loader.
Resolves #210