Bug - core - Ambiguous url('../../assets...)
Describe the problem ParcelCSS errors, because patternfly doesn't use absolute url() in css.
@parcel/transformer-css: Ambiguous url('../../assets/images/status-icon-sprite.svg#success') in custom property. Relative paths are resolved from the location the var() is used, not where the
custom property is defined. Use an absolute URL instead
/home/rp/prj/node_modules/@patternfly/react-styles/css/components/FormControl/form-control.css:122:63
121 | --pf-c-form-control--textarea--invalid--BackgroundPositionY: var(--pf-c-form-control--PaddingLeft);
> 122 | --pf-c-form-control--m-icon-sprite--success--BackgroundUrl: url("../../assets/images/status-icon-sprite.svg#success");
> | ^
123 | --pf-c-form-control--m-icon-sprite--m-warning--BackgroundUrl: url("../../assets/images/status-icon-sprite.svg#warning");
124 | --pf-c-form-control--m-icon-sprite--invalid--BackgroundUrl: url("../../assets/images/status-icon-sprite.svg#invalid");
💡 Replace with: url(/node_modules/@patternfly/react-styles/css/assets/images/status-icon-sprite.svg#success)
How do you reproduce the problem? Try to use patternfly with Parcel >= 2.4.0
Expected behavior Parcel correctly building
Is this issue blocking you? Work around is to use parcel < 2.4.0 or correctly activate PostCSS as alternative
Screenshots
What is your environment?
- OS: archlinux
- Browser chrome
- Version 102
What is your product and what release date are you targeting? patternfly 2022.7
https://parceljs.org/languages/css/#url()
Note: Only absolute paths may be used within CSS custom properties, not relative paths. This is because url() references in custom properties are resolved from the location where the var() is used, not where the custom property is defined. This means that the custom property could resolve to different URLs depending on which file it is used in. To resolve this ambiguity, use absolute paths when referencing URLs in custom properties.
Parcel makes it easy to create UI libraries that support both ES6 modules and legacy CommonJS. It would be wonderful to use Parcel with PatternFly but it is blocked by this issue.
Howdy @rp- and @jamestalton! Thanks for this issue, it's an interesting one. A handful of us met and went over this today to see what we might be able to do here. Here are some thoughts up front, and I think this would be a good topic to chat about at the next PF office hours.
I think we'll need to consider this from both the PF react and core perspectives, as the ways styles are imported are different. Generally speaking, you can import PF component styles one of 2 ways:
-
patternfly.css(at the root of the package)- The
assetsdir is in the PWD, sourl()paths to assets begin with./
- The
-
components/ComponentName/component-name.css- The
assetsdir is at the root of the package, sourl()begins with../../
- The
These relative paths are good for core/non-react users as it allows someone to place the PF package wherever they want in their document root, and the paths to the assets from either patternfly.css or the individual component CSS should resolve properly.
However (as I understand it anyways), react-styles always imports component styles from the individual component CSS, meaning the current path in the url() should always start with ../../, and the path before that should always be /node_modules/@patternfly/react-styles/css/ (the path to the package + /css). So this could be something that we offer on the react side, though I would want to verify what I've said here is actually the case, and consider the possibility of this breaking anyone. If we were able to offer this, it would likely need to be an opt-in for now, so as not to inadvertently break anyone whose use case for using styles isn't something we've considered.
Also a basic question about this error - is this more of a warning about the potential issues that could arise with using a relative path in a custom property, or is this issue is failing and/or incompatible with the way Parcel is bundling the styles?
From the core perspective, here are a few potential solutions.
-
Override the problematic CSS variables with absolute paths. This isn't a great solution as it isn't sustainable as PF may change the values and add new vars with relative paths that also need to be redefined, so it's a bit of a whack-a-mole solution, but it could work as an interim solution assuming the overrides will allow Parcel to continue and it is able to ignore the original, overriden vars with relative paths currently. For the form control component, these vars: https://github.com/patternfly/patternfly/blob/9ab78a81b9d6d63cd7795fa82b1ff673abba5332/src/patternfly/components/FormControl/form-control.scss#L189-L195
An override may look like:
.pf-c-form-control { --pf-c-form-control--m-icon-sprite--success--BackgroundUrl: url("/node_modules/@patternfly/react-styles/css/assets/images/status-icon-sprite.svg#success"); --pf-c-form-control--m-icon-sprite--m-warning--BackgroundUrl: url("/node_modules/@patternfly/react-styles/css/assets/images/status-icon-sprite.svg#warning"); --pf-c-form-control--m-icon-sprite--invalid--BackgroundUrl: url("/node_modules/@patternfly/react-styles/css/assets/images/status-icon-sprite.svg#invalid"); ... } -
Override the
$pf-global--image-pathvar and recompile our SCSS, however compiling the SCSS is not always an option and likely isn't ideal. The global image path var is part of our global variables, as you can see in the form control styles, and establishes the path for all image assets defined in the PF CSS. That var is declared here: https://github.com/patternfly/patternfly/blob/9ab78a81b9d6d63cd7795fa82b1ff673abba5332/src/patternfly/sass-utilities/scss-variables.scss#L135This is also what's needed if, for example, someone wants to override the PF global media query breakpoint vars, since those are Sass vars because we can't use CSS vars in media queries. The same idea applies here since we can't use a CSS var for the file prefix (CSS vars don't allow you to concat strings).
-
Since this is a potentially buggy way to declare paths, instead of specifying the
url()in a custom property, we could provide it as the fallback value for the property it applies to. The only downside to that would be that the custom property for theurl()PF uses could no longer be referenced by a custom property itself, though that seems like an unlikely scenario. Also, I'm not sure if this fixes the original issue. From the verbage on the Parcel url() docs, it seems like it would, but not sure if it would in practice.Instead of:
.pf-c-form-control { --pf-c-form-control--m-icon-sprite--success--BackgroundUrl: url("#{$pf-global--image-path}/status-icon-sprite.svg#success"); background-image: var(--pf-c-form-control--m-icon-sprite--success--BackgroundUrl); }We would do it like this:
.pf-c-form-control { /* Removed the --pf-c-form-control--m-icon-sprite--success--BackgroundUrl var */ background-image: var(--pf-c-form-control--m-icon-sprite--success--BackgroundUrl, url("#{$pf-global--image-path}/status-icon-sprite.svg#success")); }
Let me know what you think.
It breaks parcel as it throws an error and refuses to run. I do not have a good suggestion on the fix.
Sorry, I'm also too far away from all this CSS/SCSS stuff to give any real suggestion.
Suggestion 3 might work, but we would need to test, as it sounds as it would only fail if it is used in a custom property.
Just a follow up from our discussion at office hours:
- @jschuler is going to look into the possibility of updating
@patternfly/react-stylesto manipulate the paths used in cssurl()functions to be absolute (as Parcel has suggested in the error above). It looks like it could be added to the existing scripts that copy the PF core CSS into place. - It would be great to know if the third solution in my other comment works. If so, I think that should be fine to update on our end, at least for the text input/form control variables since that feature is beta currently. However, it looks like we also have relative paths in these two components. This could be a breaking change as those vars can be referenced now for the values they provide, and if we remove the vars, the values can no longer be referenced that way.
@jamestalton fwiw, I put together these pages to test if relative paths in CSS vars break if the components that use the vars are used on different paths. Using patternfly.css with a path to ./assets/..., or components/Component/component.css with a path to ../../assets... seems to work fine (my examples use patternfly.css), which are the use cases we support in core. I included an image at the bottom as a control just to ensure the github.io site wasn't doing something weird with the path resolution.
- https://mcoker.github.io/relative.html
- https://mcoker.github.io/dir/index.html
- https://mcoker.github.io/dir/subdir/index.html
I have this same problem for @patternfly/patternfly and I don't use @patternfly/react-styles
Hey @mingfang! Can you tell me more about your setup - how the code is structured and why the paths are failing?
I switched from parceljs to esbuild and now I'm able to use @patternfly/patternfly without any problems.
This issue has been automatically marked as stale because it has not had activity in the last 60 days. It will be closed in 30 days if no further activity occurs.
btw. this is my workaround for now:
{
"scripts": {
"build": "yarn patch-patternfly && parcel build",
"patch-patternfly": "sed -i 's/..\\/..\\/assets\\//\\/node_modules\\/@patternfly\\/react-styles\\/css\\/assets\\//g' node_modules/@patternfly/react-styles/css/components/BackgroundImage/background-image.css node_modules/@patternfly/react-styles/css/components/AboutModalBox/about-modal-box.css node_modules/@patternfly/react-styles/css/components/FormControl/form-control.css"
}
}
I am still interested in seeing this work.
@jamestalton since the ideal update may likely be a breaking change, I've added the breaking/v5 label to the issue and will try and prioritize it with the other work we're planning on working on for v5, which will release in the first half of next year.
Would you or anyone else in the thread be interested in contributing a fix? We have a v5 branch for work toward that release.
Thanks @rp- for the patch!
Looking forward to a fix in the patternfly 5.0.0 release!
I got this patch working in a windows dev environment with shx via the following in my package.json
"scripts": {
"start": "yarn patch-patternfly; parcel public/index.html",
"build": "yarn patch-patternfly; parcel build public/index.html",
"patch-patternfly": "shx sed -i 's/..\\/..\\/assets\\//\\/node_modules\\/@patternfly\\/react-styles\\/css\\/assets\\//g' node_modules/@patternfly/react-styles/css/components/BackgroundImage/background-image.css node_modules/@patternfly/react-styles/css/components/AboutModalBox/about-modal-box.css node_modules/@patternfly/react-styles/css/components/FormControl/form-control.css"
}
Here's an updated patch-patternfly to account for the changes from react-core 4.276.8 to 4.276.9
"patch-patternfly": "shx sed -i 's/..\\/..\\/assets\\//\\/node_modules\\/@patternfly\\/react-styles\\/css\\/assets\\//g' node_modules/@patternfly/react-styles/css/components/BackgroundImage/background-image.css node_modules/@patternfly/react-styles/css/components/AboutModalBox/about-modal-box.css node_modules/@patternfly/react-styles/css/components/FormControl/form-control.css node_modules/@patternfly/react-core/node_modules/@patternfly/react-styles/css/components/BackgroundImage/background-image.css node_modules/@patternfly/react-core/node_modules/@patternfly/react-styles/css/components/AboutModalBox/about-modal-box.css node_modules/@patternfly/react-core/node_modules/@patternfly/react-styles/css/components/FormControl/form-control.css node_modules/@patternfly/react-core/node_modules/@patternfly/react-styles/css/components/AboutModalBox/about-modal-box.css"