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(css): add node support for external @import #15818

Merged
merged 1 commit into from
Nov 9, 2022

Conversation

noreiller
Copy link
Contributor

Issue:
When using the CSS experiments and using @import in CSS code, the compilation fails with these error messages:

Module build failed: UnhandledSchemeError: Reading from "http://example.com/image.png" is not handled by plugins (Unhandled scheme).
Webpack supports "data:" and "file:" URIs by default.
You may need an additional plugin to handle "http:" URIs.

Changes:
This PR will handle the @import rules for node and add a dedicated test.

CSS example code:

@import "https://test.cases/path/../../../../configCases/css/external/external.css";

@webpack-bot
Copy link
Contributor

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)

@vankop
Copy link
Member

vankop commented May 13, 2022

you should @import from your code base and set public path. ( e.g. in your case https://example.com/ ). If you need assets public path you can use https://webpack.js.org/configuration/module/#rulegeneratorpublicpath

@vankop vankop closed this May 13, 2022
@noreiller
Copy link
Contributor Author

noreiller commented May 13, 2022

@vankop My bad, I have surely put a bad description and so you misunderstood the issue.
When enabling CSS experiments, node cannot parse the following CSS codes:

/* test/configCases/css/external/style2.css */
body {
	background: url(//example.com/image.png) url(https://example.com/image.png);
	background-image: url(http://example.com/image.png);
}

/* test/configCases/css/external/style.css */
@import "style2.css";
@import "https://test.cases/path/../../../../configCases/css/external/external.css";

@vankop
Copy link
Member

vankop commented May 13, 2022

if you want to handle http/https protocol urls ( bundle assets from this protocol requests ) you should just enable https://webpack.js.org/configuration/experiments/#experimentsbuildhttp

@noreiller
Copy link
Contributor Author

But it's not the wanted behavior. I don't want to bundle the remote CSS in the current compilation, I want Webpack to let me use the standard CSS behavior as described in https://developer.mozilla.org/en-US/docs/Web/CSS/@import.
The web target build already handles that thanks to the code I duplicated for the node target: https://github.com/webpack/webpack/blob/main/lib/WebpackOptionsApply.js#L141-L163.

@vankop
Copy link
Member

vankop commented May 13, 2022

yeah.. as I said in separate PR we should support ignore option.. #15821

@noreiller
Copy link
Contributor Author

Sorry, I disagree. Why don't you want to hack the source assets with magic comments when you can easily handle the code?

@vankop
Copy link
Member

vankop commented May 13, 2022

ok.. in css instead of ignoring request webpack creates ExternalModule and there is no support for node

@vankop vankop reopened this May 13, 2022
@noreiller
Copy link
Contributor Author

ok.. in css instead of ignoring request webpack creates ExternalModule and there is no support for node
Yes

My test is failing in some cases because I try to check that the @import rule is in the bundled file, have you any idea to do another way than mine?

@noreiller
Copy link
Contributor Author

@vankop @sokra Do you think this PR could be merged?

@noreiller
Copy link
Contributor Author

@vankop @sokra Do you think this PR could be merged?

up 🙄

@alexander-akait
Copy link
Member

alexander-akait commented May 31, 2022

@noreiller @sokra on vacation, he will be here in near future, your PRs on our radar, so don't worry

@noreiller
Copy link
Contributor Author

Oh ok! Thanks for the update.

@sokra sokra merged commit fcccd19 into webpack:main Nov 9, 2022
@sokra
Copy link
Member

sokra commented Nov 9, 2022

Thanks

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

5 participants