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

Asset links broken when extracting CSS and assets to different folders #691

Closed
fenomas opened this issue Feb 2, 2021 · 46 comments · Fixed by #771
Closed

Asset links broken when extracting CSS and assets to different folders #691

fenomas opened this issue Feb 2, 2021 · 46 comments · Fixed by #771

Comments

@fenomas
Copy link

fenomas commented Feb 2, 2021

  • Operating System: OSX
  • Node Version: v14.15.4
  • NPM Version: 6.14.10
  • webpack Version: 5.15.0
  • mini-css-extract-plugin Version: ^1.3.5

Expected Behavior

I'm using MiniCssExtractPlugin to extract CSS, and with default settings everything works fine. However I want to export the CSS to its own folder, separate from image assets, like this:

build/styles/bundle.css
build/assets/image.png

Per the docs, I'm doing this by passing a filename option to the plugin with the desired folder name.

Actual Behavior

When I pass in an option like: filename: 'styles/[name].css, the CSS file itself gets exported to the correct directory. However, url(..) asset links inside the CSS aren't adjusted, and wind up broken as a result.

Specifically, styles/bundle.css winds up containing references like url(assets/image.png), causing the browser to request styles/assets/image.png.

Code

The relevant code snippets look like this:

/* webpack.config.js */
    output: {
        filename: 'js/[name].[contenthash:8].js',
        path: path.resolve('.', 'build'),
        publicPath: '',
    },
    plugins: [
        new VueLoaderPlugin(),
        new HtmlWebpackPlugin({ /* .. */ }),
        new MiniCssExtractPlugin({
            filename: 'styles/[name].css' }
        ),
    ],
    module: {
        rules: [
            // ...
            {
                test: /\.png$/,
                type: 'asset/resource',
                generator: { filename: 'assets/[name][ext]' },
            },
            {
                test: /\.css$/,
                use: [ MiniCssExtractPlugin.loader, 'css-loader' ],
            },
/* /src/foo/bar.css */
.whatever {
    background-image: url('~path/to/image.png');
}

After running webpack, the CSS winds up with broken asset links:

/* styles/bundle.css */
.whatever {
    background-image: url('assets/image.png');
        /* should be: url('../assets/image.png') or similar */
}

How Do We Reproduce?

By building a project with configuration as described above (in production mode).


Is there some setting or workaround supporting this use case?

Thanks!

@alexander-akait
Copy link
Member

Please do not ignore how we can reproduce, sorry won't fix without it, I don't know your output options and how CSS handles, maybe bug in vue-loader, maybe bug in code and resolving options, do you use dev server or just build, I don't know, I will reopen and help you when you fill out ### How Do We Reproduce?

@fenomas
Copy link
Author

fenomas commented Feb 2, 2021

Hi, I didn't "ignore" the option, I just couldn't think of anything to say there that wasn't already in the other sections. I'll add the info you asked. I'm building normally - I don't know if the issue happens when serving via dev-server, as (per the docs) I don't use CSS extraction in that case.

@alexander-akait
Copy link
Member

@AndyHall So please provide full configuration and part of code, or better minimum reproducible test repo, I am really want to help, but many issues without good examples and I can't help

@alexander-akait
Copy link
Member

Feel free to ping me when it will be ready

@fenomas
Copy link
Author

fenomas commented Feb 2, 2021

@alexander-akait Sorry, I don't know what you're asking for. The config I posted is already as minimal as I can make it, and it took a fair bit of effort. Are you saying you want the full config I'm using, including everything that isn't related to this issue?

@alexander-akait
Copy link
Member

@AndyHall I can't reproduce problem with your configuration, so yes, I need fully, you can just create simple reproducible test repo and include part of configuration where the problem reproduced

@alexander-akait
Copy link
Member

Why you have publicPath: '',? I think you want publicPath: '/',

@fenomas
Copy link
Author

fenomas commented Feb 2, 2021

@alexander-akait

Why you have publicPath: '',? I think you want publicPath: '/',

Wouldn't that mean the generated resources will all have hard paths relative to server root? I'm using '' so that resources are linked relative from the html file.

you can just create simple reproducible test repo and include part of configuration where the problem reproduced

For a closed issue?

@alexander-akait
Copy link
Member

For a closed issue?

It was closed, because you don't provide enough information to help, if we will keep opened all issues where developers doesn't provide enough formation, here will be trash and will be hard for developers found real problem(s), I will help and/or reopen it if you provide steps to reproduce

@fenomas
Copy link
Author

fenomas commented Feb 2, 2021

I'm trying in good faith to report an issue. There's no call to be referring to it as trash or not a real problem.

Here is a minimal repro. Config, code, and steps to reproduce are same as posted previously.

@alexander-akait
Copy link
Member

I'm trying in good faith to report an issue. There's no call to be referring to it as trash or not a real problem.

I understand you, but try to understand me too, I got very many issues every day and when developer provide only part of code, it requires me to spend efforts on recreating configuration and code to reproduce the problem and most of times not problems (from my experience it is 99% of problems), so other developers won't get help or fixes/features because I will spend most of the time trying to reproduce the problem

@fenomas
Copy link
Author

fenomas commented Feb 2, 2021

I understand how maintaining works. But if policy for this module is to close any issue without a repo, just say that in the readme or the issue template.

@alexander-akait
Copy link
Member

Yep, I will update

@alexander-akait
Copy link
Member

I need think about it more, now you can use publicPath: '../', as workaround

@fenomas
Copy link
Author

fenomas commented Feb 3, 2021

Setting publicPath: '../' only fixes the paths affected by this issue - it breaks the other paths in my project.

@alexander-akait
Copy link
Member

alexander-akait commented Feb 3, 2021

@AndyHall Yes, in ideal we should https://github.com/webpack/webpack/blob/f9a2c275855027e828983e731ce98189fb7b6d81/lib/util/identifier.js#L311, but we have url only after building, don't have good idea how we can fix it, I need think about it...

Note for me:
Other example https://github.com/xenobytezero/sass-loader-github-bug

@fenomas
Copy link
Author

fenomas commented Feb 3, 2021

@alexander-akait Take your time, there's no rush. I'm already working around the issue myself; I only posted this issue for others who might encounter it.

@karlvr
Copy link
Contributor

karlvr commented Mar 23, 2021

I believe I am having the same issue... I am including .less files in a JavaScript component; the .less file is in a path ./js/components/less/example.less and it references the image as url(../../../img/example.jpg). My publicPath option to the MiniCssExtractPlugin loader is return path.relative(path.dirname(resourcePath), context) + '/', and I end up url(../../../img/example.jpg) in the output CSS.

However, because the component that's including the less is getting included from a file in ./js/example.js, that relative path is no longer accurate. It seems that MiniCssExtractPlugin knows the filename of the eventual output (using the filename option). I am going to look into this issue as well to see if it's something I can improve.

@karlvr
Copy link
Contributor

karlvr commented Mar 23, 2021

It seems that we should be able to compute a relative path in renderContentAsset based on the context of the CssModule and the path that we're rendering the chunk to... by passing that info for each url out to a helper function, like the publicPath one...

@alexander-akait
Copy link
Member

If somebody have solutions right now, feel free to send a PR

@karlvr
Copy link
Contributor

karlvr commented Mar 23, 2021

Working on it. I must say it's hard to get ones head around publicPath in all its ways!

I have something that works for me... I'm wondering if it will do the trick for everyone.

MiniCSSExtractPlugin currently supports the publicPath = "auto" by simply setting publicPath = "". I wonder if that's intentional or because there was no better solution. The docs suggest that "auto" means that publicPath will be automatically determined. I'm hoping that setting it to "" means we have ¯_(ツ)_/¯ed here, and therefore my solution co-opts "auto" in the hopes that it then also becomes default behaviour and just does the right thing!

My solution sets publicPath = "'/__MINI_CSS_EXTRACT_PLUGIN__/'" when the publicPath option is set to "auto", and then we replace that in the plugin at which point we know the context-relative path of the output file.

In renderContentAsset(compiler, compilation, chunk, modules, requestShortener) we add:

const publicPath = path.relative(path.dirname(chunk.name), "");
content = content.replace(/\/__MINI_CSS_EXTRACT_PLUGIN__/g, publicPath);

So now the "auto" mode means that we compute a relative path from our output file to the publicPath / root and then use that for each asset in our output CSS.

I'm going to prepare a PR. I'd love some feedback if this approach seems suitable.

@karlvr karlvr mentioned this issue Mar 23, 2021
6 tasks
@karlvr
Copy link
Contributor

karlvr commented Mar 23, 2021

I've also published a test module that we're using internally at https://npmjs.com/@cactuslab/mini-css-extract-plugin that might be the easiest way to test if this resolves your issue @AndyHall

@fenomas
Copy link
Author

fenomas commented Mar 24, 2021

@karlvr I tried your plugin in my reproducing repo, but webpack's output didn't change at all. Was I supposed to do something besides swapping the main plugin for yours?

@karlvr
Copy link
Contributor

karlvr commented Mar 24, 2021

@AndyHall do you have publicPath set to anything? And what version of Webpack are you using? I am using 5. If you're using 4, please try setting publicPath to "auto" in the loader options for mini-css-extract-plugin.

@fenomas
Copy link
Author

fenomas commented Mar 24, 2021

@karlvr I'm building the minimal-reproduction repo linked up-thread. Versions and settings are in the repo.

Edit: in that repo I swapped the plugin module to yours, and tried building with and without the publicPath: 'auto' setting, but webpack's CSS output didn't change.

@karlvr
Copy link
Contributor

karlvr commented Mar 24, 2021

@AndyHall thanks Andy... I have reproduced the fault in your minimal repo. I made some assumptions about the chunk name and that it would reflect where the output was going. It does in my case as I use a relative filename for my outputs...

So, all we need to know is where we're planning to write the file that we're outputting. Is that possible?

@karlvr
Copy link
Contributor

karlvr commented Mar 24, 2021

@AndyHall OK, please upgrade to 1.3.10 of @cactuslab/mini-css-extract-plugin. This now looks at the output filename template to work out any additional relative paths to consider. In your case, that is styles/[name].css so we get an extra ../ which is what we need.

In order to make it work with your repo I removed the publicPath line from webpack.config.js (as "auto" is the default), upgraded the plugin, and updated the require statement in webpack.config.js, then ran npm run build.

I am really straying into an area of interacting with the webpack API that I'm not at all familiar with. I hope there's a function to render a filename template given a chunk... I've just regexed the name into the template in case the name contains path components!

@fenomas
Copy link
Author

fenomas commented Mar 25, 2021

@karlvr Is removing publicPath required for your fix to work?

I tried the steps you suggested and they fix the CSS link, but per the original post, publicPath: '' is part of the repro steps for this issue. Removing it breaks other stuff in my actual project.

(I could be missing something, I don't know anything about webpack internals and wasn't able to follow what your fix is doing.)

@karlvr
Copy link
Contributor

karlvr commented Mar 25, 2021

@AndyHall Good question. Removing publicPath (which sets it to its default in Webpack 5, which is now "auto") is what my patch currently uses to activate its changes. But I guess the same fix is required for any relative publicPath.

What is the intended meaning of setting publicPath to ""? Could you detail what breaks in your project when projectPath is "auto" instead?

Anyway… it would be possible to activate this patch on any relative publicPath including "". We know where the .css file is output so we can always compute a relative path to the root of the context. In fact we don't ever even need publicPath. Maybe our thinking changes to "we'll use publicPath only if it's absolute, otherwise we can compute a relative path easily thank you very much, and relative paths always work consistently in CSS files, relative to the CSS file itself"

Super easy to change the patch to this way of thinking. Is it too much or flawed @alexander-akait? I'm starting to think it's actually essential.

@alexander-akait
Copy link
Member

Anyway… it would be possible to activate this patch on any relative publicPath including "". We know where the .css file is output so we can always compute a relative path to the root of the context. In fact we don't ever even need publicPath. Maybe our thinking changes to "we'll use publicPath only if it's absolute, otherwise we can compute a relative path easily thank you very much, and relative paths always work consistently in CSS files, relative to the CSS file itself"

You are right here, in long term we should avoid using publicPath, you need specify it only when you need absolute urls, like CDN or specific env.

@karlvr
Copy link
Contributor

karlvr commented Mar 28, 2021

@alexander-akait Perhaps a good first step is to take publicPath values "auto" and "" and "take over" and compute the relative URL ourselves, and see where that leads? I think that's what I've done in the PR... I'll try to improve the applying of the template this week.

@fenomas
Copy link
Author

fenomas commented Mar 31, 2021

What is the intended meaning of setting publicPath to ""?

you need specify it only when you need absolute urls, like CDN or specific env

Hi, just to clarify here I'm using publicPath: '' to mean that linked assets should be linked relative to the location of the html file loading them, not from a CDN/etc. The intention is that if you build the repo I posted, the contents of the build folder could then be copied into any directory on a server.

@alexander-akait
Copy link
Member

publicPath: '' and publicPath: 'auto' is not the same, publicPath: 'auto' should do relative assets, publicPath: '' is just empty publicPath

@karlvr
Copy link
Contributor

karlvr commented Mar 31, 2021

@AndyHall removing publicPath or setting it to auto will achieve what you want to achieve. We have to do something in order to make the uris to assets correct in the CSS we output, so I've just pushed a patch that treats publicPath '' in the same way, and I believe it will achieve what you want to achieve.

@AndyHall you mention that removing publicPath (effectively setting it to 'auto' on webpack 5) breaks other things. Would you be able to provide details of the other things it breaks? I think that would help me, at least, understand how best to resolve it here.

Also, please upgrade to @cactuslab/mini-css-extract-plugin@1.3.11 as I have included the patch to handle the '', and please let me know if this now has your whole webpack project working.

@fenomas
Copy link
Author

fenomas commented Apr 6, 2021

Hi @karlvr , regarding "removing publicPath will achieve what you want to achieve", I don't know what you mean. Having publicPath: '' is part of the config for my project, separate from the issue we're talking about here.

However with that said, using the 1.3.11 version with my current config appears to fix the issue for me. I can extract the CSS output into a folder, and url(..) links in the CSS are no longer broken. I have no idea about the inner workings, but outwardly LGTM!

@alexander-akait
Copy link
Member

Yep, it is good fix, need couple test and we can merge it

@karlvr
Copy link
Contributor

karlvr commented Apr 6, 2021

publicPath: '' and publicPath: 'auto' is not the same, publicPath: 'auto' should do relative assets, publicPath: '' is just empty publicPath

Yes, that's literally absolutely correct, however I think the intention of the two is the same. I think webpack 5 just made it explicit. The default was empty; the default is now auto.

An empty publicPath is broken for CSS when the CSS is located out of the root. But people expect it to work as I think they want it to mean "use relative paths and make it work", which I think is synonymous with auto.

That's why I'm curious what breaks by adopting publicPath auto (or removing publicPath and thus using the default value of auto). My expectation is that everything just works, and where it doesn't there lie more bugs to fix!

@alexander-akait
Copy link
Member

Anyway, can you add couple tests for your PR?

@devthiagocaetano
Copy link

devthiagocaetano commented Apr 12, 2021

@AndyHall OK, please upgrade to 1.3.10 of @cactuslab/mini-css-extract-plugin. This now looks at the output filename template to work out any additional relative paths to consider. In your case, that is styles/[name].css so we get an extra ../ which is what we need.

In order to make it work with your repo I removed the publicPath line from webpack.config.js (as "auto" is the default), upgraded the plugin, and updated the require statement in webpack.config.js, then ran npm run build.

This worked for me!! Thanks a lot 🇧🇷
obs: I never used publicPath, so just installed the npm package and changed the require.

@karlvr When I added filename: 'assets/css/[name].css', in MiniCssExtractPlugin (wich refers to yours), my dist css looses the images again. Is there a way to work around this??
I can give u more infos, but you guys have done a great job so far, and I don't want to waste your time on this if would be unnecessary

@thomasfw
Copy link

thomasfw commented Jun 2, 2021

@karlvr would you be able to resolve the linter errors so your PR can be merged? Thanks in advance!

@badrylin
Copy link

badrylin commented Jun 12, 2021

@alexander-akait
Copy link
Member

Fixed by #772, if you use before publicPath: '' or publicPath: '../' or something similar to get relative url() in your CSS files (same for JS files), remove these lines, webpack v5 by default uses publicPath: 'auto' and mini-css-extract-plugin fully support it since v2. Shorty: webpack v5 and mini-css-extract-plugin generate relative URLs by default or when you use publicPath: 'auto', also it supported by webpack-dev-server, I strongly recommend use auto value by default (exception - when you use CDN)

@fenomas
Copy link
Author

fenomas commented Jun 30, 2021

mini-css-extract-plugin fully support it since v2

What is "v2" here? Do you mean a future release?

@alexander-akait
Copy link
Member

WIP on release right now

@alexander-akait
Copy link
Member

Done https://github.com/webpack-contrib/mini-css-extract-plugin/releases/tag/v2.0.0, the next goal first class HMR supporting, including splitChunks

@johnmagbag1995
Copy link

Fixed mine using Webpack 5 Resource assets instead of using file-loader.

{
    test: /\.(png|jpe?g|gif)$/i,
    type: 'asset/resource',
},
{
    test: /\.css$/i,
    use: [
      {
        loader: MiniCssExtractPlugin.loader,
      },
      'css-loader',
      'postcss-loader',
    ],
},

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