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

css url() is not relative to publicPath #1390

Open
Edorka opened this issue Aug 31, 2020 · 5 comments
Open

css url() is not relative to publicPath #1390

Edorka opened this issue Aug 31, 2020 · 5 comments

Comments

@Edorka
Copy link

Edorka commented Aug 31, 2020

Do you want to request a feature or report a bug?

Seems like a bug
What is the current behaviour?

Even after setting on config a new publicPath and this is being observed on js files:

import previous from './preact.config.js';

export default function (config, env, helpers) {
    config.output.publicPath = '/wp-content/themes/nsp/';
    return config;
}

The CSS files with sentences like background-image: url('/assets/images/nsp-complete.png');

the output css bundles returns these rules with the same path

What is the expected behaviour?

IMHO honoring public path should be background-image: url(/wp-content/themes/nsp'/assets/images/nsp-complete.png');`

Currently I'm updating all references using a small script:

egrep -lRZ '/assets' wp-theme/ | xargs -0 -l sed -i -e 's/\/assets/\/wp-content\/themes\/nsp\/assets/g'

Thanks for your time reading this.

Please paste the results of preact info here.

Environment Info:
System:
OS: Linux 5.4 elementary OS 5.1.7 Hera
CPU: (4) x64 Intel(R) Core(TM) i5-6200U CPU @ 2.30GHz
Binaries:
Node: 12.18.3 - /usr/bin/node
Yarn: 1.22.4 - /usr/bin/yarn
npm: 6.14.6 - /usr/bin/npm
Browsers:
Chrome: 84.0.4147.125
npmPackages:
preact: ^10.3.2 => 10.4.7
preact-cli: ^3.0.0 => 3.0.1
preact-render-to-string: ^5.1.4 => 5.1.10
preact-router: ^3.2.1 => 3.2.1

@rschristian
Copy link
Member

rschristian commented Sep 10, 2020

Shouldn't be a bug but instead the lack of a feature. Might be missing on purpose though, have come across a few threads which suggest security issues with this. Not knowledgeable enough to say anything about the subject myself.

You can however get this behaviour by setting the publicPath option of MiniCssExtractPlugin. Take a look here for the documentation on how to do such a thing.

Edit: I don't think setting publicPath is what you want to do at all. My bad.

@Edorka
Copy link
Author

Edorka commented Sep 11, 2020

Finally after trying all webpack plugins related to css url() on npmjs finally got a solution after reading https://stackoverflow.com/questions/46580441/webpack-transform-replace-css-url

this solution required installing 'string-replace-loader'

import previous from './preact.config.js';

export default function (config, env, helpers) {
    const publicPath = '/wp-content/themes/nsp/';
    config.output.publicPath = publicPath;
    config.module.rules.push({
        test: /\.css/,
        loader: 'string-replace-loader',
        options: {
          search: /url\('\/assets\/(.*)?'\)/i, 
          replace: (_match, resource) => `url('${publicPath}assets/${resource}')`,
          flags: 'g',
        }
      });
    previous(config, env, helpers);
}

It's pretty barbarian but works and maybe helps someone

@developit
Copy link
Member

@ryanchristian4427 - I wonder if it'd be worth having CLI infer the publicPath value for MiniCssExtract from the config?

Something like this to replace these lines:

					use: [
						isWatch ? 'style-loader' : {
							loader: MiniCssExtractPlugin.loader,
							options: {
								get publicPath() {
									// `config` is a new variable assigned to the returned object from L113
									return config.output && config.output.publicPath;
								}
							}
						},
						{

@rschristian
Copy link
Member

rschristian commented Sep 13, 2020

I'd say so. Can't think of a situation in which a user would want to change the publicPath of assets but exclude CSS from that, besides the theoretical. Making this easier to set would be a good thing.

Though this isn't really related to the issue on second look. What @Edorka here wants isn't to set the publicPath at all, but to prefix URLs. Didn't catch this on my first read through, so my first comment isn't really relevant.

While we can certainly add the functionality you wrote up and it will help those wanting to set a public path, it won't solve this issue, and I'm not sure if any tool out there does? Setting the public path to /example/path won't turn /assets/x.png into /example/path/assets/x.png like desired. It'll instead turn a relative URL that paths to an actual resource like ../assets/x.png into /example/path/[hash].png.

@Edorka
Copy link
Author

Edorka commented Sep 16, 2020

I don't know if it would worth the bytes to opena a new issue: This also affects manifest.json icon paths

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants