Next #760

alexander-akait · 2018-08-24T14:02:19Z

What kind of change does this PR introduce?
bugfix, feature, refactoring

Did you add tests for your changes?

Yes

If relevant, did you update the README?

Yep

Summary

Rewrite css-loader

Does this PR introduce a breaking change?

Yes

alexander-akait · 2018-08-24T14:05:11Z

Looks node on appveyor generate other hashes when linux, very weird, maybe problem in https://github.com/css-modules/generic-names (we should update loader-utils to latest version, already require npm access)

alexander-akait · 2018-08-24T14:15:05Z

what the hell with CI 😕

michael-ciniawsky · 2018-08-24T18:45:24Z

Ideally we extract the needed interpolateName code from loader-utils and use it in generic-names directly or add a new method to one of the ICSS Utils and use that instead. The dependency on loader-utils is not needed

michael-ciniawsky · 2018-08-24T18:47:19Z

Hashes are not platform consistent to my knowledge, but maybe it was fixed in newer version of loader-utils

alexander-akait · 2018-08-27T10:11:24Z

@michael-ciniawsky wip on updating

alexander-akait · 2018-08-27T16:18:32Z

/cc @michael-ciniawsky CI green (problem in deps also fixed)

joepie91 · 2018-08-27T16:53:15Z

Just gave this a quick testrun, and I ran into the following error:

ERROR in ./test.css
Module build failed (from ./node_modules/mini-css-extract-plugin/dist/loader.js):
TypeError: Cannot read property 'foo' of undefined
    at Object.../3rdparty/css-loader-upstream/dist/cjs.js?!./test.css (/home/sven/projects/3rdparty/css-loader-upstream/dist/cjs.js??ref--5-1!/home/sven/projects/icss-loader-test/test.css:786:251)
    at __webpack_require__ (/home/sven/projects/3rdparty/css-loader-upstream/dist/cjs.js??ref--5-1!/home/sven/projects/icss-loader-test/test.css:684:30)
    at /home/sven/projects/3rdparty/css-loader-upstream/dist/cjs.js??ref--5-1!/home/sven/projects/icss-loader-test/test.css:751:93
    at Object.<anonymous> (/home/sven/projects/3rdparty/css-loader-upstream/dist/cjs.js??ref--5-1!/home/sven/projects/icss-loader-test/test.css:754:10)
    at Module._compile (/home/sven/projects/icss-loader-test/node_modules/v8-compile-cache/v8-compile-cache.js:178:30)
    at exec (/home/sven/projects/icss-loader-test/node_modules/mini-css-extract-plugin/dist/loader.js:55:10)
    at childCompiler.runAsChild (/home/sven/projects/icss-loader-test/node_modules/mini-css-extract-plugin/dist/loader.js:133:14)
    at compile (/home/sven/projects/icss-loader-test/node_modules/webpack/lib/Compiler.js:296:11)
    at hooks.afterCompile.callAsync.err (/home/sven/projects/icss-loader-test/node_modules/webpack/lib/Compiler.js:553:14)
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/home/sven/projects/icss-loader-test/node_modules/tapable/lib/HookCodeFactory.js:24:12), <anonymous>:24:1)
    at AsyncSeriesHook.lazyCompileHook [as _callAsync] (/home/sven/projects/icss-loader-test/node_modules/tapable/lib/Hook.js:35:21)
    at compilation.seal.err (/home/sven/projects/icss-loader-test/node_modules/webpack/lib/Compiler.js:550:30)
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/home/sven/projects/icss-loader-test/node_modules/tapable/lib/HookCodeFactory.js:24:12), <anonymous>:6:1)
    at AsyncSeriesHook.lazyCompileHook [as _callAsync] (/home/sven/projects/icss-loader-test/node_modules/tapable/lib/Hook.js:35:21)
    at hooks.optimizeAssets.callAsync.err (/home/sven/projects/icss-loader-test/node_modules/webpack/lib/Compilation.js:1294:35)
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/home/sven/projects/icss-loader-test/node_modules/tapable/lib/HookCodeFactory.js:24:12), <anonymous>:6:1)
    at AsyncSeriesHook.lazyCompileHook [as _callAsync] (/home/sven/projects/icss-loader-test/node_modules/tapable/lib/Hook.js:35:21)
    at hooks.optimizeChunkAssets.callAsync.err (/home/sven/projects/icss-loader-test/node_modules/webpack/lib/Compilation.js:1285:32)
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/home/sven/projects/icss-loader-test/node_modules/tapable/lib/HookCodeFactory.js:24:12), <anonymous>:6:1)
    at AsyncSeriesHook.lazyCompileHook [as _callAsync] (/home/sven/projects/icss-loader-test/node_modules/tapable/lib/Hook.js:35:21)
    at hooks.additionalAssets.callAsync.err (/home/sven/projects/icss-loader-test/node_modules/webpack/lib/Compilation.js:1280:36)
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/home/sven/projects/icss-loader-test/node_modules/tapable/lib/HookCodeFactory.js:24:12), <anonymous>:6:1)
    at AsyncSeriesHook.lazyCompileHook [as _callAsync] (/home/sven/projects/icss-loader-test/node_modules/tapable/lib/Hook.js:35:21)
    at hooks.optimizeTree.callAsync.err (/home/sven/projects/icss-loader-test/node_modules/webpack/lib/Compilation.js:1276:32)
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/home/sven/projects/icss-loader-test/node_modules/tapable/lib/HookCodeFactory.js:24:12), <anonymous>:6:1)
    at AsyncSeriesHook.lazyCompileHook [as _callAsync] (/home/sven/projects/icss-loader-test/node_modules/tapable/lib/Hook.js:35:21)
    at Compilation.seal (/home/sven/projects/icss-loader-test/node_modules/webpack/lib/Compilation.js:1213:27)
    at hooks.make.callAsync.err (/home/sven/projects/icss-loader-test/node_modules/webpack/lib/Compiler.js:547:17)
    at _err0 (eval at create (/home/sven/projects/icss-loader-test/node_modules/tapable/lib/HookCodeFactory.js:24:12), <anonymous>:11:1)
    at _addModuleChain (/home/sven/projects/icss-loader-test/node_modules/webpack/lib/Compilation.js:1064:12)
 @ ./index.jsx 3:10-31

Full testcase can be found at https://git.cryto.net/joepie91/icss-loader-test. This compiled 'correctly' with css-loader 1.0.0 except for it producing the wrong class names due to #562, so presumably the CSS itself is correct.

I tested it against next by cloning the css-loader repository, switching to the next branch, npm installing it, and yarn linking it into my test repository. I didn't use npm link or install-from-git due to a litany of outstanding npm bugs. I've verified that the error really does occur in next and not in 1.0.0.

joepie91 · 2018-08-27T18:08:09Z

On closer inspection, it also seems to not support CSS Modules? Or at least the composes syntax doesn't seem to do anything, and ends up as-is in my build product (when removing the :import block to avoid the compilation error).

There doesn't seem to be any option to enable support for this either, and the option for specifying a class name ident format has also disappeared in this release.

michael-ciniawsky · 2018-08-27T20:24:01Z

For composes the loader likely needs to add it's own resolver, postcss-icss-composes is syntax only (to my knowledge)

alexander-akait · 2018-08-28T09:28:00Z

@joepie91 old css modules doesn't work with next css modules (ICSS) in some cases, you can find example with css modules in README

alexander-akait · 2018-08-28T09:57:25Z

@joepie91 also you have invalid code you try to import class, but no export :export{foo: foo}

joepie91 · 2018-08-28T14:36:09Z

Hmm, the ICSS spec suggests that you should only specify exports for exported literals, not for class names?

An exportedValue does not need to be quoted, it is already treated as a literal string.

(Emphasis mine.)

How does that combine with having to explicitly export classes? Given that those are presumably not literal strings.

alexander-akait · 2018-08-28T14:58:40Z

@joepie91 to avoid big size of locals you should export/import only what you need, css-loader@1 works same

alexander-akait · 2018-12-04T15:57:42Z

All in master

alexander-akait requested a review from michael-ciniawsky August 24, 2018 14:02

alexander-akait force-pushed the next branch 2 times, most recently from 3766e86 to 93ae399 Compare August 24, 2018 15:13

alexander-akait force-pushed the next branch 12 times, most recently from 8f7507d to 2c2c757 Compare August 27, 2018 15:39

alexander-akait mentioned this pull request Aug 27, 2018

feat: support imported classes in selectors (postcss-icss) #562

Closed

2 tasks

alexander-akait mentioned this pull request Aug 28, 2018

sourcemap doesn't work as expected with css-modules #738

Closed

alexander-akait force-pushed the next branch from b4e8250 to 407b651 Compare September 27, 2018 21:07

alexander-akait mentioned this pull request Oct 1, 2018

Respect exclude option when handling url #779

Closed

alexander-akait mentioned this pull request Oct 10, 2018

fix(loader): trim unquoted import urls #783

Merged

evilebottnawi and others added 13 commits October 29, 2018 17:47

refactor: drop css modules

9e58086

feat: support icss

5b1633a

chore(deps): update

f4c7041

chore: migrate on npm audit

f3ee4de

fix: handle uppercase media query

d55a34c

chore(deps): update

5dc22b9

chore: update webpack-default

9981fe6

test: fix

d79a1bf

refactor: import at-rule

9673de4

refactor: icss stuff

2afea5f

refactor: url

2502fbc

refactor: stuff

2a41654

docs: update

cb4c028

alexander-akait force-pushed the next branch from 407b651 to cb4c028 Compare October 29, 2018 14:49

alexander-akait added 7 commits October 30, 2018 18:35

feat: support postcss-modules

049a280

test: pre loader

5403db7

fix: coverage paths

430ede3

feat: import/module/export message api

6ee472d

style: use it instead test

779de4d

fix: stuff

2dcac11

refactor: tests

3a22a62

alexander-akait force-pushed the next branch from e40485e to 3a22a62 Compare October 30, 2018 22:00

alexander-akait added 2 commits October 31, 2018 15:04

fix: url with query

ec1f17d

test: mini css extract plugin

829d7d5

alexander-akait closed this Dec 4, 2018

alexander-akait deleted the next branch December 4, 2018 15:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Next #760

Next #760

alexander-akait commented Aug 24, 2018 •

edited

Loading

alexander-akait commented Aug 24, 2018 •

edited

Loading

alexander-akait commented Aug 24, 2018

michael-ciniawsky commented Aug 24, 2018

michael-ciniawsky commented Aug 24, 2018 •

edited

Loading

alexander-akait commented Aug 27, 2018

alexander-akait commented Aug 27, 2018

joepie91 commented Aug 27, 2018

joepie91 commented Aug 27, 2018

michael-ciniawsky commented Aug 27, 2018 •

edited

Loading

alexander-akait commented Aug 28, 2018 •

edited

Loading

alexander-akait commented Aug 28, 2018

joepie91 commented Aug 28, 2018

alexander-akait commented Aug 28, 2018 •

edited

Loading

alexander-akait commented Dec 4, 2018

Next #760

Next #760

Conversation

alexander-akait commented Aug 24, 2018 • edited Loading

alexander-akait commented Aug 24, 2018 • edited Loading

alexander-akait commented Aug 24, 2018

michael-ciniawsky commented Aug 24, 2018

michael-ciniawsky commented Aug 24, 2018 • edited Loading

alexander-akait commented Aug 27, 2018

alexander-akait commented Aug 27, 2018

joepie91 commented Aug 27, 2018

joepie91 commented Aug 27, 2018

michael-ciniawsky commented Aug 27, 2018 • edited Loading

alexander-akait commented Aug 28, 2018 • edited Loading

alexander-akait commented Aug 28, 2018

joepie91 commented Aug 28, 2018

alexander-akait commented Aug 28, 2018 • edited Loading

alexander-akait commented Dec 4, 2018

alexander-akait commented Aug 24, 2018 •

edited

Loading

alexander-akait commented Aug 24, 2018 •

edited

Loading

michael-ciniawsky commented Aug 24, 2018 •

edited

Loading

michael-ciniawsky commented Aug 27, 2018 •

edited

Loading

alexander-akait commented Aug 28, 2018 •

edited

Loading

alexander-akait commented Aug 28, 2018 •

edited

Loading