fix: stricter `@import` tolerance #593

swernerx · 2017-08-16T18:31:42Z

I accidentally found this issue when using size-limit on an exported library where we kept the import-normalize from https://www.npmjs.com/package/postcss-normalize inside the exported library.

Issue is that Webpack currently throws on the simple statement as it tries to parse it as a normal import.

To fix I added the failing test first, then fixed the regexp which matches the import statements and made the url handling generally more tolerant.

michael-ciniawsky · 2017-08-16T19:08:50Z

lib/processCss.js

@@ -42,12 +42,12 @@ var parserPlugin = postcss.plugin("css-loader-parser", function(options) {
 		}

 		if(options.import) {
-			css.walkAtRules(/import/i, function(rule) {
+			css.walkAtRules(/^import$/i, function(rule) {


/^@import$/i

@michael-ciniawsky walkAtRules already walks only on @something.

The at-sign is not needed here, right.

michael-ciniawsky · 2017-08-16T19:09:18Z

lib/processCss.js

 				var values = Tokenizer.parseValues(rule.params);
 				var url = values.nodes[0].nodes[0];
-				if(url.type === "url") {
+				if(url && url.type === "url") {


I'd like to get the same error handling about malformed URLs for non-existing URLs. This was actually the original error, where I got something like can't look up "type" of undefined.

@swernerx can your add tests for this, thanks!

This would be a new kind of test which tests for throwing errors. We do not do this currently in CSS loader it seems... at least not in the importTest.js file.

@swernerx we should add tests or avoid this fix here (move to separate issue or PR)

@swernerx oh no 😄 your test was right with @import, we should tests about url && url.type === "url"

That's what I did. Without the new handling it should just throw some hard error.

michael-ciniawsky · 2017-08-16T19:09:28Z

lib/processCss.js

 					url = url.url;
-				} else if(url.type === "string") {
+				} else if(url && url.type === "string") {


alexander-akait

Good! See comments @michael-ciniawsky

codecov · 2017-08-16T20:07:24Z

Codecov Report

Merging #593 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master     #593   +/-   ##
=======================================
  Coverage   98.63%   98.63%           
=======================================
  Files          10       10           
  Lines         367      367           
  Branches       87       87           
=======================================
  Hits          362      362           
  Misses          5        5

Impacted Files	Coverage Δ
lib/processCss.js	`97.98% <100%> (ø)`	⬆️

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 b92c941...be3eef5. Read the comment docs.

…r a new css-loader release fixing webpack-contrib/css-loader#593

This Pull Request updates dependency [css-loader](https://github.com/webpack-contrib/css-loader) from `^0.28.11` to `^1.0.0` <details> <summary>Release Notes</summary> ### [`v1.0.0`](https://github.com/webpack-contrib/css-loader/blob/master/CHANGELOG.md#100httpsgithubcomwebpack-contribcss-loadercomparev02811v100-2018-07-06) [Compare Source](webpack-contrib/css-loader@v0.28.11...v1.0.0) ##### BREAKING CHANGES * remove `minimize` option, use [`postcss-loader`](https://github.com/postcss/postcss-loader) with [`cssnano`](https://github.com/cssnano/cssnano) or use [`optimize-cssnano-plugin`](https://github.com/intervolga/optimize-cssnano-plugin) plugin * remove `module` option, use `modules` option instead * remove `camelcase` option, use `camelCase` option instead * remove `root` option, use [`postcss-loader`](https://github.com/postcss/postcss-loader) with [`postcss-url`](https://github.com/postcss/postcss-url) plugin * remove `alias` option, use [`resolve.alias`](https://webpack.js.org/configuration/resolve/) feature or use [`postcss-loader`](https://github.com/postcss/postcss-loader) with [`postcss-url`](https://github.com/postcss/postcss-url) plugin * update `postcss` to `6` version * minimum require `nodejs` version is `6.9` * minimum require `webpack` version is `4` #### [0.28.11](webpack-contrib/css-loader@v0.28.10...v0.28.11) (2018-03-16) ##### Bug Fixes * **lib/processCss:** don't check `mode` for `url` handling (`options.modules`) ([#698](`webpack-contrib/css-loader#698)) ([c788450](webpack-contrib/css-loader@c788450)) #### [0.28.10](webpack-contrib/css-loader@v0.28.9...v0.28.10) (2018-02-22) ##### Bug Fixes * **getLocalIdent:** add `rootContext` support (`webpack >= v4.0.0`) ([#681](`webpack-contrib/css-loader#681)) ([9f876d2](webpack-contrib/css-loader@9f876d2)) #### [0.28.9](webpack-contrib/css-loader@v0.28.8...v0.28.9) (2018-01-17) ##### Bug Fixes * ignore invalid URLs (`url()`) ([#663](`webpack-contrib/css-loader#663)) ([d1d8221](webpack-contrib/css-loader@d1d8221)) #### [0.28.8](webpack-contrib/css-loader@v0.28.7...v0.28.8) (2018-01-05) ##### Bug Fixes * **loader:** correctly check if source map is `undefined` ([#641](`webpack-contrib/css-loader#641)) ([0dccfa9](webpack-contrib/css-loader@0dccfa9)) * proper URL escaping and wrapping (`url()`) ([#627](`webpack-contrib/css-loader#627)) ([8897d44](webpack-contrib/css-loader@8897d44)) #### [0.28.7](webpack-contrib/css-loader@v0.28.6...v0.28.7) (2017-08-30) ##### Bug Fixes * pass resolver to `localsLoader` (`options.alias`) ([#601](`webpack-contrib/css-loader#601)) ([8f1b57c](webpack-contrib/css-loader@8f1b57c)) #### [0.28.6](webpack-contrib/css-loader@v0.28.5...v0.28.6) (2017-08-30) ##### Bug Fixes * add support for aliases starting with `/` (`options.alias`) ([#597](`webpack-contrib/css-loader#597)) ([63567f2](webpack-contrib/css-loader@63567f2)) #### [0.28.5](webpack-contrib/css-loader@v0.28.4...v0.28.5) (2017-08-17) ##### Bug Fixes * match mutliple dashes (`options.camelCase`) ([#556](`webpack-contrib/css-loader#556)) ([1fee601](webpack-contrib/css-loader@1fee601)) * stricter `[@import]` tolerance ([#593](`webpack-contrib/css-loader#593)) ([2e4ec09](webpack-contrib/css-loader@2e4ec09)) #### [0.28.4](webpack-contrib/css-loader@v0.28.3...v0.28.4) (2017-05-30) ##### Bug Fixes * preserve leading underscore in class names ([#543](`webpack-contrib/css-loader#543)) ([f6673c8](webpack-contrib/css-loader@f6673c8)) #### [0.28.3](webpack-contrib/css-loader@v0.28.2...v0.28.3) (2017-05-25) ##### Bug Fixes * correct plugin order for CSS Modules ([#534](`webpack-contrib/css-loader#534)) ([b90f492](webpack-contrib/css-loader@b90f492)) #### [0.28.2](webpack-contrib/css-loader@v0.28.1...v0.28.2) (2017-05-22) ##### Bug Fixes * source maps path on `windows` ([#532](`webpack-contrib/css-loader#532)) ([c3d0d91](webpack-contrib/css-loader@c3d0d91)) #### [0.28.1](webpack-contrib/css-loader@v0.28.0...v0.28.1) (2017-05-02) ##### Bug Fixes * allow to specify a full hostname as a root URL ([#521](`webpack-contrib/css-loader#521)) ([06d27a1](webpack-contrib/css-loader@06d27a1)) * case insensitivity of [@import] ([#514](`webpack-contrib/css-loader#514)) ([de4356b](webpack-contrib/css-loader@de4356b)) * don't handle empty [@import] and url() ([#513](`webpack-contrib/css-loader#513)) ([868fc94](webpack-contrib/css-loader@868fc94)) * imported variables are replaced in exports if followed by a comma ([#504](`webpack-contrib/css-loader#504)) ([956bad7](webpack-contrib/css-loader@956bad7)) * loader now correctly handles `url` with space(s) ([#495](`webpack-contrib/css-loader#495)) ([534ea55](webpack-contrib/css-loader@534ea55)) * url with a trailing space is now handled correctly ([#494](`webpack-contrib/css-loader#494)) ([e1ec4f2](webpack-contrib/css-loader@e1ec4f2)) * use `btoa` instead `Buffer` ([#501](`webpack-contrib/css-loader#501)) ([fbb0714](webpack-contrib/css-loader@fbb0714)) ##### Performance Improvements * generate source maps only when explicitly set ([#478](`webpack-contrib/css-loader#478)) ([b8f5c8f](webpack-contrib/css-loader@b8f5c8f)) --- </details> --- This PR has been generated by [Renovate Bot](https://renovatebot.com).

swernerx added 3 commits August 16, 2017 20:19

Added failed test for @import-normalize

1b68e75

Fixed import handling not matching import like constructs.

3b25b93

Made tolerant for null match on urls inside import

3087559

michael-ciniawsky reviewed Aug 16, 2017

View reviewed changes

michael-ciniawsky added priority: 4 (important) pr_action: Review semver: Patch severity: 3 (broken) status: Pending type: Bug labels Aug 16, 2017

michael-ciniawsky added this to the 0.28.5 milestone Aug 16, 2017

michael-ciniawsky requested a review from alexander-akait August 16, 2017 19:11

michael-ciniawsky changed the title ~~Fix/low tolerance import~~ fix: stricter @import tolerance Aug 16, 2017

alexander-akait reviewed Aug 16, 2017

View reviewed changes

Added request test case

be3eef5

michael-ciniawsky approved these changes Aug 16, 2017

View reviewed changes

michael-ciniawsky added pr_action: Merge ready status: Approved and removed pr_action: Review status: Pending labels Aug 16, 2017

swernerx added a commit to sebastian-software/edge-core that referenced this pull request Aug 17, 2017

Disabled size-limit for test success for now as I am still waiting fo…

4e6be56

…r a new css-loader release fixing webpack-contrib/css-loader#593

joshwiens merged commit 2e4ec09 into webpack-contrib:master Aug 17, 2017

michael-ciniawsky modified the milestone: 0.28.6 Aug 17, 2017

michael-ciniawsky removed priority: 4 (important) pr_action: Merge ready severity: 3 (broken) labels Aug 17, 2017

swernerx added a commit to sebastian-software/edge that referenced this pull request Oct 22, 2017

Disabled size-limit for test success for now as I am still waiting fo…

ba7872a

…r a new css-loader release fixing webpack-contrib/css-loader#593

snyk-bot mentioned this pull request Aug 16, 2019

[Snyk] Upgrade css-loader from 0.23.1 to 0.28.11 DriveClutch/sellpy-dockerfiles#4

Closed

snyk-bot mentioned this pull request Jan 8, 2020

[Snyk] Upgrade css-loader from 0.17.0 to 0.28.11 timscharfenort/kibana#3

Open

This was referenced Jan 22, 2020

[Snyk] Upgrade css-loader from 0.23.1 to 0.28.11 anyulled/Learn-Redux-Starter-Files#1

Open

[Snyk] Upgrade css-loader from 0.24.0 to 0.28.11 anyulled/react-skeleton#2

Closed

This was referenced Feb 23, 2020

[Snyk] Upgrade css-loader from 0.26.4 to 0.28.11 saurabharch/stitch-examples#2

Open

[Snyk] Upgrade css-loader from 0.23.1 to 0.28.11 saurabharch/guesstimate-app#3

Open

[Snyk] Upgrade css-loader from 0.23.1 to 0.28.11 saurabharch/CodeGenius#2

Open

snyk-bot mentioned this pull request Mar 7, 2020

[Snyk] Upgrade css-loader from 0.23.1 to 0.28.11 yowainwright/react-workshop#4

Closed

This was referenced Apr 2, 2020

[Snyk] Upgrade css-loader from 0.26.4 to 0.28.11 fiverr/rollout_dashboard#48

Open

[Snyk] Upgrade css-loader from 0.19.0 to 0.28.11 rangle/react-day-picker#2

Open

snyk-bot mentioned this pull request Apr 24, 2020

[Snyk] Upgrade css-loader from 0.16.0 to 0.28.11 walmartlabs/bolt-standard-flux#3

Open

This was referenced May 4, 2020

[Snyk] Upgrade css-loader from 0.23.1 to 0.28.11 Barnstorm-Online/votetracker#1

Open

[Snyk] Upgrade css-loader from 0.23.1 to 0.28.11 React-Junkie/react-coffee-and-friends#2

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: stricter `@import` tolerance #593

fix: stricter `@import` tolerance #593

swernerx commented Aug 16, 2017

michael-ciniawsky Aug 16, 2017

alexander-akait Aug 16, 2017 •

edited

swernerx Aug 16, 2017

michael-ciniawsky Aug 16, 2017

swernerx Aug 16, 2017

alexander-akait Aug 16, 2017

swernerx Aug 16, 2017

alexander-akait Aug 16, 2017

swernerx Aug 16, 2017

alexander-akait Aug 16, 2017

swernerx Aug 16, 2017

michael-ciniawsky Aug 16, 2017

alexander-akait left a comment •

edited

codecov bot commented Aug 16, 2017 •

edited

fix: stricter @import tolerance #593

fix: stricter @import tolerance #593

Conversation

swernerx commented Aug 16, 2017

Choose a reason for hiding this comment

alexander-akait Aug 16, 2017 • edited

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

Choose a reason for hiding this comment

alexander-akait left a comment • edited

Choose a reason for hiding this comment

codecov bot commented Aug 16, 2017 • edited

Codecov Report

fix: stricter `@import` tolerance #593

fix: stricter `@import` tolerance #593

alexander-akait Aug 16, 2017 •

edited

alexander-akait left a comment •

edited

codecov bot commented Aug 16, 2017 •

edited