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.

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

fix: ignore invalid URLs (url()) #663

Merged
merged 3 commits into from Jan 17, 2018

Conversation

kgram
Copy link
Contributor

@kgram kgram commented Jan 16, 2018

What kind of change does this PR introduce?

Closes #659

Did you add tests for your changes?
Test added for dependency exporting an empty object and null.

If relevant, did you update the README?
Not relevant

Summary

#627 introduced an assumption about all dependencies exporting strings. To avoid including font-files, some users use null-loader, which effectively exports an empty object.

This PR checks for non-strings, restoring previous functionality for all non-string dependencies.

While this PR would enable previous usage patterns, I hope someone can come up with a better solution. The users are relying on url([object Object]) being invalid CSS to avoid making requests for font files. This seems fragile in the long run.

Ideally, a null/undefined value (or what we here know should be a null value, {}) should result in no request being made. As far as I know, this requires the entire url(...) block be replaced with none, and I'm still not sure this would be valid at all times. This would be a complicated solution.

Does this PR introduce a breaking change?

No.

@@ -16,7 +16,7 @@ function getEvaluated(output, modules) {
return require("../lib/url/escape");
if(module.indexOf("-!/path/css-loader!") === 0)
module = module.substr(19);
if(modules && modules[module])
if(modules && module in modules)
Copy link
Member

Choose a reason for hiding this comment

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

Why in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@evilebottnawi: To allow for falsy modules such as { './module': null } to still be resolved.

Copy link
Member

Choose a reason for hiding this comment

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

@kgram thanks for clafiry 👍

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Looks good! One question, see above.

@kgram
Copy link
Contributor Author

kgram commented Jan 17, 2018

@evilebottnawi: Do you know what is wrong with the travis-build? It looks like the processes were aborted halfway through when looking at the logs.

@alexander-akait
Copy link
Member

@kgram do not pay attention (legacy), main build doing in ci/circleci

@michael-ciniawsky michael-ciniawsky changed the title fix: restore null-loader support fix: ignore invalid URLs (url()) Jan 17, 2018
Copy link
Member

@michael-ciniawsky michael-ciniawsky left a comment

Choose a reason for hiding this comment

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

@kgram Thx for taking care of this 👍

@michael-ciniawsky
Copy link
Member

Released in v0.28.9 🎉

eliperelman pushed a commit to neutrinojs/neutrino that referenced this pull request Jul 10, 2018
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#&#8203;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`) ([#&#8203;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`) ([#&#8203;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()`) ([#&#8203;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` ([#&#8203;641](`webpack-contrib/css-loader#641)) ([0dccfa9](webpack-contrib/css-loader@0dccfa9))
* proper URL escaping and wrapping (`url()`) ([#&#8203;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`)  ([#&#8203;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`) ([#&#8203;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`) ([#&#8203;556](`webpack-contrib/css-loader#556)) ([1fee601](webpack-contrib/css-loader@1fee601))
* stricter `[@import]` tolerance ([#&#8203;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 ([#&#8203;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 ([#&#8203;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` ([#&#8203;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 ([#&#8203;521](`webpack-contrib/css-loader#521)) ([06d27a1](webpack-contrib/css-loader@06d27a1))
* case insensitivity of [@&#8203;import] ([#&#8203;514](`webpack-contrib/css-loader#514)) ([de4356b](webpack-contrib/css-loader@de4356b))
* don't handle empty [@&#8203;import] and url() ([#&#8203;513](`webpack-contrib/css-loader#513)) ([868fc94](webpack-contrib/css-loader@868fc94))
* imported variables are replaced in exports if followed by a comma ([#&#8203;504](`webpack-contrib/css-loader#504)) ([956bad7](webpack-contrib/css-loader@956bad7))
* loader now correctly handles `url` with space(s) ([#&#8203;495](`webpack-contrib/css-loader#495)) ([534ea55](webpack-contrib/css-loader@534ea55))
* url with a trailing space is now handled correctly ([#&#8203;494](`webpack-contrib/css-loader#494)) ([e1ec4f2](webpack-contrib/css-loader@e1ec4f2))
* use `btoa` instead `Buffer` ([#&#8203;501](`webpack-contrib/css-loader#501)) ([fbb0714](webpack-contrib/css-loader@fbb0714))
##### Performance Improvements

* generate source maps only when explicitly set ([#&#8203;478](`webpack-contrib/css-loader#478)) ([b8f5c8f](webpack-contrib/css-loader@b8f5c8f))

---

</details>




---

This PR has been generated by [Renovate Bot](https://renovatebot.com).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

v0.28.8 regression: Relative urls in @font-face throw exception TypeError: url.replace is not a function
3 participants