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

Initial work towards native css support #14894

Merged
merged 26 commits into from Dec 17, 2021
Merged

Initial work towards native css support #14894

merged 26 commits into from Dec 17, 2021

Conversation

sokra
Copy link
Member

@sokra sokra commented Dec 2, 2021

What kind of change does this PR introduce?
experiment

Did you add tests for your changes?
yes

Does this PR introduce a breaking change?
no

What needs to be documented once your changes are merged?

@webpack-bot
Copy link
Contributor

webpack-bot commented Dec 2, 2021

For maintainers only:

  • This needs to be documented (issue in webpack/webpack.js.org will be filed when merged)
  • This needs to be backported to webpack 4 (issue will be created when merged)

@alexander-akait
Copy link
Member

alexander-akait commented Dec 2, 2021

CSS allow to write url like:

div {
  background: url('./img\
\
\
\
-other-part.png');
}

We normalize it in css-loader.

Here example of most of all urls - https://github.com/webpack-contrib/css-loader/blob/master/test/fixtures/url/url.css, we can add them here (all test passed in css-loader).

Also CSS allows to have interesting things in url:

div {  
  background: url(./img\'img.png);  
  background: url("./img%27%27%27img.png"); 
}

Test case for @import https://github.com/webpack-contrib/css-loader/blob/master/test/fixtures/import/import.css, but some of them broken for snapshots testing, broken things do not changed and keeping as is in CSS without resolving.

Also CSS allow to do circular imports (they just ignored because already loaded):

file.css

@import "file.css";

@sokra
Copy link
Member Author

sokra commented Dec 3, 2021

Thanks. I'll add these as tests.

Some of them are a bit weird: image-set("./img1x.png" 1x);
Seem like using url() is optional here... That makes it a bit more complicated...

@sokra
Copy link
Member Author

sokra commented Dec 3, 2021

And I found another fancy thing in the spec:

background-image: image('sprites.svg#xywh=40,0,20,20', black)

https://developer.mozilla.org/en-US/docs/Web/CSS/image/image()

@alexander-akait
Copy link
Member

background-image: image('sprites.svg#xywh=40,0,20,20', black)

Yep, but in real world I have not seen anyone use this, url is shorter 😄

@sokra
Copy link
Member Author

sokra commented Dec 3, 2021

Yep, but in real world I have not seen anyone use this

It seem to be new and not yet supported:

image

It can show a fallback colors and crop images, so that's capability that url() doesn't have.

lib/asset/RawDataUrlModule.js Outdated Show resolved Hide resolved
* @param {Hash} hash hash that will be modified
* @param {UpdateHashContext} updateHashContext context for updating hash
*/
updateHash(hash, { module }) {}
Copy link
Member

Choose a reason for hiding this comment

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

should we do here same as #14857

? Template.asString([
"if (link.src.indexOf(window.location.origin + '/') !== 0) {",
Template.indent(
`link.crossOrigin = ${JSON.stringify(crossOriginLoading)};`
Copy link
Member

Choose a reason for hiding this comment

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

interesting small improvement #10064 (comment) possible here

lib/css/CssModulesPlugin.js Outdated Show resolved Hide resolved
return pos;
};
walkCssTokens(source, {
url: (input, start, end, contentStart, contentEnd) => {
Copy link
Member

Choose a reason for hiding this comment

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

maybe we need a hooks here (at least in webpack@6)

@webpack-bot
Copy link
Contributor

I've created an issue to document this in webpack/webpack.js.org.

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.

None yet

4 participants