Open Artur- opened 2 years ago
Artur-
commented
2 years ago Oh we actually see this in many many of our tests now also when trying to upgrade from 3.0.4 to 3.0.7: https://github.com/vaadin/flow/pull/14356. Which is a bit weird because in the other project, it shows up also with 3.0.4
Artur-
commented
2 years ago Perhaps related to #9381 or some other import meta PR @sapphi-red ?
sapphi-red
commented
2 years ago What is the problem here? Because
"@font-face{font-family:Line Awesome Brands;font-style:normal;font-weight:400;font-display:auto;src:url("+new URL('la-brands-400.c0e32387.eot', import.meta.url).href+");"is in JS, it will be evaluated as something like
"@font-face{font-family:Line Awesome Brands;font-style:normal;font-weight:400;font-display:auto;src:url(https://example.com/assets/la-brands-400.c0e32387.eot);"which is a valid CSS.
This import.meta.url is used for relative base, so using base: './' will be a important for creating reproduction.
Artur-
commented
2 years ago The problem is that it is not evaluated but the JS code remains in the final string
patak-dev
commented
2 years ago Just a detail, but previewHacks should already be a string here, no? There is no need for toString
injectGlobalCss(previewHacks.toString());It would help a lot to be able to get a reproduction, I wonder if one of your plugins is getting in the way. If it isn't evaluated, it means that the CSS code didn't end up like:
" ... "+new URL().hred+" ... "when you are importing previewHacks, that is quite strange
Artur-
commented
2 years ago The code in the output JS is like
const Rl = O`@font-face{font-family:Line Awesome Brands;font-style:normal;font-weight:400;font-display:auto;src:url("+new URL("la-brands-400.c0e32387.eot",import.meta.url).href+");src:url("+new URL("la-brands-400.c0e32387.eot?#iefix",import.meta.url).href+") format("embedded-opentype"),url("+new URL("la-brands-400.ff70c9bc.woff2",import.meta.url).href+") format("woff2"),url("+new URL("la-brands-400.14c63377.woff",import.meta.url).href+") format("woff"),url("+new URL("la-brands-400.fbc98702.ttf",import.meta.url).href+") format("truetype"),url("+new URL("la-brands-400.4da18191.svg#lineawesome",import.meta.url).href+") format("svg")}So as the CSS is wrapped inside
O`css`breaking out using "+new URL will obviously not work
I suspect the wrapping is done by rollup-plugin-postcss-lit
patak-dev
commented
2 years ago Interesting, this looks like something that could be fixed in rollup-plugin-postcss-lit. It isn't safe to wrap code in this way. At one point, we had used
` ... ${new URL().href} ... `For our wrapping, but that also may have issues depending on their (I assume) regex based replacing 🤔 If it is this plugin, you should be able to create a reproduction and maybe check if they are willing to patch their plugin.
patak-dev
commented
2 years ago Thinking about this some more, since we are replacing the URL in renderChunk, it is not really safe for Vite to assume that no other plugin has transformed it from "..." to a template string. So I think this issue falls on our side.
I see two options:
` ... ${new URL().href} ... `This should be safer in general, maybe we could do this change and release it in 3.1 to avoid disruption (prob. out in two weeks, but you could start using a beta sooner)
__VITE_ASSET__ being replaced directly in renderChunk.Leaning towards 2. if we can find a good implementation.
Artur-
commented
2 years ago Here is a reproduction case https://github.com/Artur-/vite-vaadin-problem7
sapphi-red
commented
2 years ago If it's using tagged template literal (css`a`) and not a normal template literal (css(`a`)), are we able to preserve the meaning? (I'm not sure if this is the case though.)
function t(...args){console.log(args)}
t`aa` // [['aa']]
t`a${'url'}a` // [['a', 'a'], 'url']
t(`a${'url'}a`) // ['aurla']For example, if rollup-plugin-postcss-lit is transforming (a) to (b), transforming (b) to (c) might break it.
// (a)
export default ".foo { background: url('./foo') }"
// (b)
export default css`.foo { background: url('./foo') }`
// (c)
export default css`.foo { background: url('${new URL('./foo', import.meta.url).href}') }`
web-padawan
commented
2 years ago Tried rollup-plugin-lit-css and it has the same problem when using the following config in the project linked above:
import litcss from 'rollup-plugin-lit-css';
export default {
base: './',
plugins: [
{
...litcss({include: '**/*.css*'}),
enforce: 'post'
}
],
}That plugin use string-to-template-literal instead of RegEx based replacing.
web-padawan
commented
2 years ago Compared the code produced by two plugins I tested, and both have src:url(__VITE_ASSET__b7ef2cd1__) in the output.
The only real difference in the output is format("woff") vs format(\\"woff\\"); which seems irrelevant.
rollup-plugin-postcss-lit@font-face{font-family:Roboto;font-style:normal;font-display:swap;font-weight:400;src:url(__VITE_ASSET__b7ef2cd1__) format("woff2"),url(__VITE_ASSET__e41533d5__) format("woff")}rollup-plugin-lit-css@font-face{font-family:Roboto;font-style:normal;font-display:swap;font-weight:400;src:url(__VITE_ASSET__b7ef2cd1__) format(\\"woff2\\"),url(__VITE_ASSET__e41533d5__) format(\\"woff\\");}It should be possible to workaround this by making the URL wrapped as Vite expects it so instead of rewriting in the plugin from
@font-face{src:url(__VITE_ASSET__c0e32387__)}to
cssTag`@font-face{src:url(__VITE_ASSET__c0e32387__)}`it would rewrite to
cssTag`@font-face{src:url(${unsafeCSSTag("__VITE_ASSET__c0e32387__")})Then __VITE_ASSET__ is quoted with " like Vite expects..
patak-dev
commented
2 years ago Do you have a reference for src:url @Artur-?
@sapphi-red do you have an example where it will break? I know that the named template string could potentially do anything, but I assume that if they see a URL interpolated they must respect it. Maybe they will be adding custom logic to be able to update it later? But I think the resulting CSS should be well-formed.
Artur-
commented
2 years ago For the record, this resolves the issue for us https://github.com/umbopepato/rollup-plugin-postcss-lit/pull/54/files
patak-dev
commented
2 years ago For the record, this resolves the issue for us umbopepato/rollup-plugin-postcss-lit#54 (files)
Oh, interesting. Great that they were so responsive. They will also need to guard against __VITE_PUBLIC_ASSET__. I still think that we should be guarding against this in Vite core, so let's keep discussing in this issue.
Artur-
commented
2 years ago Well... I created that PR :)
sapphi-red
commented
2 years ago I think this will break if we inject new URL(foo) without wrapping with unsafeCSS. (stackblitz)
If you must use an expression in a css literal that is not itself a css literal, and you are confident that the expression is from a fully trusted source such as a constant defined in your own code, then you can wrap the expression with the unsafeCSS function: lit docs
Artur-
commented
2 years ago unsafeCSS is not at all a great solution, far from it but it allows us to move forward before this is fixed properly
manolo
commented
2 years ago Is this fixed, or still an issue ?
Artur-
commented
1 year ago This issue reappears when trying to upgrade from Vite 5 beta 9 to beta 10. Not sure what has changed
Artur-
commented
1 year ago In the test logs (https://github.com/vaadin/flow/actions/runs/6627641560/job/18003126076) there are requests for files like
Error message in browser log: [2023-10-24T13:47:41.126Z] [SEVERE] http://localhost:8888/+new%20URL('fa-solid-900-9yKDp_qD.woff2',%20import.meta.url).href+ - Failed to load resource: the server responded with a status of 404 (Not Found)@Artur- beta10 upgrades rollup to v4. rollup v4 uses some additional characters for the reference id. I guess you need to change this line like I did here. https://github.com/vaadin/flow/blob/66cec6601f458f514e42cbe4f9ea279f58469db7/flow-server/src/main/resources/plugins/rollup-plugin-postcss-lit-custom/rollup-plugin-postcss-lit.js#L31 https://github.com/vitejs/vite/pull/14508/files#diff-91776e7c6039d23a070162f02a69cd46046a2095bd5ecb384ae9e27f2ea5288fL30-R31
Artur-
commented
1 year ago Thanks! That did it and we are 🍏 📗 🟢 💚 with Vite 5 beta 10
Messa1
commented
9 months ago Is there any fix for that problem? Local all work as I need. When I deploy and build i got a 404 error. downloadable font: download failed (font-family: "Font Awesome 6 Free" style:normal weight:900 stretch:100 src index:0): status=2147746065 source: https://DOMAIN.COM/dist/webfonts/fa-solid-900.woff2
package.json
{
"name": "kirby-vite",
"main": "src/index.js",
"type": "module",
"scripts": {
"vite": "vite",
"dev": "concurrently \"npm:server\" \"npm:vite\"",
"server": "php -S localhost:8888 -t public server.php",
"build": "vite build",
"preview": "npm run build && npm run server"
},
"author": "arnoson",
"license": "MIT",
"devDependencies": {
"concurrently": "^8.2.2",
"glob": "^10.3.10",
"vite": "^5.1.5",
"vite-plugin-kirby": "^5.3.0"
},
"dependencies": {
"@fortawesome/fontawesome-free": "^6.5.1",
"mburger-webcomponent": "^3.1.4",
"mmenu-js": "^9.3.0",
"sass": "^1.71.1",
"uikit": "^3.19.1"
}
}index.scss
// Theme
@import "theme/variables-theme.scss";
@import "uikit/src/scss/variables-theme.scss";
@import "uikit/src/scss/mixins-theme.scss";
@import '@fortawesome/fontawesome-free/scss/fontawesome.scss';
@import '@fortawesome/fontawesome-free/scss/brands.scss';
@import '@fortawesome/fontawesome-free/scss/solid.scss';
@import '@fortawesome/fontawesome-free/scss/regular.scss';
@import "theme/_import.scss";
@import "mmenu-js/src/mmenu.scss";vite.config.php
<?php
// This is an auto-generated file. Please avoid making changes here.
// Configure your settings in the "vite.config.js" file instead.
return [
'rootDir' => 'src',
'outDir' => 'public/dist',
'assetsDir' => 'assets',
'legacy' => false
];
Describe the bug
One of our project which works fine with Vite 2.x has code like
where
preview-hacks.csscontainsThe
line-awesome.min.cssfile containsWith Vite 3.0.0-3.0.7 the CSS string
previewHacks.toString()contains in dev modebut when built for production it contains
Reproduction
Reproduces in a private Vaadin project. So far I have been unable to trigger it in a simple project. @patak-dev has access to the project though
System Info
Used Package Manager
npm
Logs
No response
Validations