Skip to content
This repository has been archived by the owner on Mar 17, 2021. It is now read-only.

Retain query string in the final file output path #364

Closed
Powell-v2 opened this issue Feb 14, 2020 · 23 comments · Fixed by #366
Closed

Retain query string in the final file output path #364

Powell-v2 opened this issue Feb 14, 2020 · 23 comments · Fixed by #366

Comments

@Powell-v2
Copy link

Feature Proposal

Hi there,

Currently any query string gets stripped away from the output path: path/to/image.webp?q=/width_50/quality_50 becomes path/to/image.webp. As far as I can tell, there is neither a placeholder representing query string (e.g. [name].[hash:8].[ext][querystring]), nor an option allowing to retain it.

Seems like a straightforward change as resource's query string is available under this.resourceQuery property, however I'm not really sure why it's removed in the first place and how it might affect existing projects.

I'll be happy to submit a PR - just need to gain the context first.

Thank you!

Feature Use Case

Major use case for this feature is custom processing applied to images post-deploy by Image CDN services. It's typically done via query strings.

@alexander-akait
Copy link
Member

alexander-akait commented Feb 14, 2020

It can't be fixed on file-loader side, already discussed many times, sorry, it is the internal mechanism. It is used for arguments for other loaders. You can write a custom plugin/loader if you need to save some of them or add new

@anikethsaha
Copy link

i dont see either any use case of this feature as nonscriptable files cant process those query string.

example: how a .png will process the queries ?

SO its not suitable and not workable

@alexander-akait
Copy link
Member

Server side handling, but I recommend setup webpack to ignore them, because they are not real static assets

@kamermans
Copy link

The primary use case is Image CDNs. Influencers like Google are recommending people use Image CDNs. These CDNs allow the developer to adjust the image on-the-fly using URL parameters. Every file that file-loader loads will ultimately be served via HTTP, and usually by a server that will gladly ignore a URL parameter for a static file. Webpack and file-loader are ubiquitous in the front end web development space, and it makes total sense to encourage the widest adoption of best-practices by enabling this in the most widely-used components.

Previously submitted issues/PRs that address this problem were closed, and quite possibly for a good reason. It's easy to see how this feature may be requested because someone is simply doing something incorrectly, or using an anti-pattern we all want to avoid.

I see this issue as different because the landscape has changed. None of us (web developers) want to deal with optimizing our assets, which is why projects like webpack, file-loader, image-loader, etc, exist to begin with. Today there are more than a dozen Image CDNs that are happy to optimize images on-the-fly, but are made more powerful if the developer can pass some hints to the server about how it should handle the image. For example, perhaps you want a higher-quality image on your homepage hero element and higher compression on the other images to maintain good performance.

It seems the arguments boil down to two points:

  1. It can't be fixed on file-loader side / it's not workable
    It can be made possible for users to do in only one line. If you simply pass resourceQuery to the publicPath function here, users will be able to alter their asset's public paths, appending the query string if they so choose.
  2. Its not suitable / I don't see any use case of this feature as nonscriptable files can't process those query string
    Hopefully my explanation above has made this clear.

@alexander-akait
Copy link
Member

alexander-akait commented Feb 14, 2020

Do you know what some proxies in your path into internet (from a client to a server) still not respect query parameters for cache invalidation and still cache a result, you can try it manually and found a lot of them? So most of CDN use urls like https://code.jquery.com/jquery-3.4.1.slim.js where you have a version as part of pathname. Just for a information.

Using query string for static assets doesn't make sense, it is static assets. Do you want to process dynamic server requests as static? Doesn’t it seem strange to you? You specify examples when developers use absolute urls in source code on CDN, yes, we don't touch absolute urls. I’m even interested to see your configuration. And I’m ready to show you how to configure everything correctly so as not to encounter similar problems.

Honestly, this has already been discussed many times and showed the anti-patterns that you talk about above and how to fix them.

The resourceQuery property for internal purpose. Yes you can implement own loader and implement any logic.

file-loader and url-loader will be deprecated for webpack@5 in favor asset type mode.

Feel free to leave feedback.

@kamermans
Copy link

kamermans commented Feb 14, 2020

First, you are super-responsive and I definitely appreciate that! I have helped maintain many open source projects and it's really hard to keep on top of the non-stop requests, so thank you!

Do you know what some proxies in your path into internet (from a client to a server) still not respect query parameters for cache invalidation

Yes, great point. This is why some Image CDNs like ImageEngine (full disclosure, I a co-founder) allow directives in the URL instead of just the query string. This problem is much less common these days (compared to when I started working on internet architecture 25 years ago) though, and usually it's not proxies between you and the remote site, but rather it's the remote site itself. In my use case, the remote site is an Image CDN and they have full control over the incoming request.

Using query string for static assets doesn't make sense, it is static assets. Do you want to process dynamic server requests as static? Doesn’t it seem strange to you?

No, it doesn't seem strange to me at all. Client/server content-negotiation is already serving your static assets dynamically. When you put a JPEG on your site and use a CDN, it's probably being served as a WEBP to Chrome clients whether you like it or not. When you pack your JS bundle and throw it up on a server, it's probably being served as gzip or brotli. When a powerful Image CDN serves your images, it may reduce the resolution or quality, remove EXIF data and superfluous color profiles and serve it in a different format. From the point of the user, this asset is still static insofar as every user "sees the same thing", but under the hood, companies like mine are "trimming the fat" off your assets.

Honestly, this has already been discussed many times and showed the anti-patterns that you talk about above and how to fix them.

It's totally possible that I'm trying to fix this in the wrong place. I'm not a frontend/webpack guru, so I admit that I may be missing an obvious route to solving this problem, and I really appreciate your offer to suggest how I might fix this problem.

Also, thanks for the note about webpack 5 moving to the asset type, that was totally off my radar!

An example of my configuration/scenario is this: my customers use frameworks like Vue and React and have written their sites in idiomatic ways, like using resource-loaders. Here's an example in Vue:

<img src="@/assets/images/foo-diagram.png"/>

This person would like to ensure that this image is resized automatically as negotiated by the client and server, but should not exceed 640px wide, and I would like to tell them to do this:

<img src="@/assets/images/foo-diagram.png?imgeng=w_auto,640"/>

This doesn't work, however, because the query string is dropped.
The alternatives are all less-desirable:

  1. Tell the customer to move all their images to a public, static folder and change all the references to all their images in all places to point to the new location (labor-intensive, not idiomatic).
  2. Create plugins for every framework (Vue, React, etc) that introduces a new component besides <img> that could be used instead (labor-intensive, probably misses all the default loaders for things like versioning, etc)
  3. Tell my customers that they are "doing it wrong" and that they should create several versions of each image at different resolutions and qualities so they don't need the parameters (I think this might be the quintessence of an anti-pattern).
  4. Create a new webpack plugin like file-loader that does everything exactly the same as file-loader but doesn't discard the query string, or appends it in the publicPath processing (perhaps this is the best alternative, I am looking into it; it will be annoying to have to track file-loader so perfectly, and we'll need to find a way to copy the main file-loader config that the user is already using).
  5. Create a new webpack plugin that gracefully fixes the problem some way - I'm open to suggestions and happy to drop a donation your way to steer me in the right direction :)

@alexander-akait
Copy link
Member

@kamermans Ok, let’s think about how we can solve this here, and then transfer this solution to webpack@5 asset type to avoid problem in future.

Can you create a minimal easy example? Because having a code and an approximate structure on hand will make it easier for us to communicate.

You provide example:

<img src="@/assets/images/foo-diagram.png?imgeng=w_auto,640"/>

Should be the imgeng argument was static? Or it is set up using JS? Or it is do developer on when writing code?

@kamermans
Copy link

Thanks for the response!

Should be the imgeng argument was static? Or it is set up using JS? Or it is do developer on when writing code?

Yes, the query string should be static, otherwise the browser's speculative preloader will load the original version and the one modified by JS.

Browsers get around this problem via responsive images and client hints.

I've put together a minimal example within a typical framework by creating a "hello world" Vue CLI project and editing the template so there is one <img> resource (using srcset, which is working fine in webpack) and one CSS background-image (with two variants via media queries). Both images are being included via file-loader.

Here is the example repo, with the <img> tag highlighted: https://github.com/kamermans/webpack-image-cdn-example/blob/master/src/App.vue#L13-L16

I have included examples in the "traditional" method (creating multiple image variants for each breakpoint) and the "new" method (using an Image CDN to deal with the variants).

Here is the live version of the page, which is automatically built by netlify:
https://distracted-ritchie-1f0417.netlify.com/

I am using a CDN to serve the assets by setting the publicPath in vue.config.js, which gets passed to webpack.

The generated code in live site will be a little strange - all the images in the srcset are the same, and so are both of the background images. This is because the query string is stripped, otherwise they would be real, different variants.

This might not be what you had in mind for a "minimal example", so please let me know if you want me to put something together with just webpack or something. I thought it might be helpful to see it in the context of a running minimal application.

@alexander-akait
Copy link
Member

I think I understand your idea. I need to discussion with other developers how best to solve this. I’ll try to get it today, feel free to ping me 👍

@alexander-akait
Copy link
Member

alexander-akait commented Feb 19, 2020

Just interesting, why do not implement:

<img src="//cdn.domain.com/w_360/webp/https://mysite.xyz/dog.png" alt="My dog">

@alexander-akait
Copy link
Member

alexander-akait commented Feb 19, 2020

For local development developers use this:

<img src="./w_360/webp/https://mysite.xyz/dog.png" alt="My dog">

For production developers set up publicPath: "https://cdn.domain.com"

@kamermans
Copy link

Just interesting, why do not implement:

<img src="//cdn.domain.com/w_360/webp/https://mysite.xyz/dog.png" alt="My dog">

This would work (at least for my CDN), but most people CNAME their domain (mysite.xyz) to their CDN (cdn.domain.com) so they just need to change the hostname from which their assets are served, for example, by changing the publicPath


For local development developers use this:

<img src="./w_360/webp/https://mysite.xyz/dog.png" alt="My dog">

For production developers set up publicPath: "https://cdn.domain.com"

That is an interesting idea as well, but I don't see how it would work for local development unless you physically create the path ./w_360/webp/https://mysite.xyz and put dog.png in it.

I think the main reason people can't do this in practice is that:

  1. They have a strong incentive to use their own custom domain name to serve the assets.
  2. They generally want to change only the hostname of the assets (not the path) because of other, less flexible systems that they are using.
  3. Most Image CDNs do not support path-based directives. Off the top of my head, only our CDN (ImageEngine) supports this, and only using the "URL Prefixing" mode where you append the complete URL to the end of your CDN URL. I used this method because I didn't feel like creating a proper account for this example, but maybe I'll do that so you can see how this would typically look.

@alexander-akait
Copy link
Member

alexander-akait commented Feb 19, 2020

Thanks for the information. I think we can implement this using name: '[path][name].[ext][query]', also provide query params to the Function notation. It should be easy to implement, no additional options and simple for setup. + set up publicPath: "https://cnd.domain.com/"

@kamermans
Copy link

I've updated my example site to use the CDN in a more typical way.

You will see that my assets are being served from lxrue9qg.cdn.imgeng.in, which is the CDN. The CDN then grabs them from the site (distracted-ritchie-1f0417.netlify.com) and optimizes them.

@kamermans
Copy link

kamermans commented Feb 19, 2020

Ok, great, I had prototyped this by adding the resourceQuery to the Function notation and that worked fine in Webpack 4.

I don't really think there is any downside to including the query string - the majority of people won't use the query string, and if they do, it won't do anything unless they are on some sort of CDN.

@alexander-akait
Copy link
Member

alexander-akait commented Feb 19, 2020

The default value of name will be without [query] for file-loader, because we need do breaking change for this, releasing new major version is unnecessary because in our long term developers should migrate on asset type (I think we will respect [query] by default for asset type)

@kamermans
Copy link

For reference, when [query] is available, this is how it can be set in a Vue CLI application:

// vue.config.js
module.exports = {
    // Use ImageEngine to serve our assets in production
    publicPath: process.env.NODE_ENV === 'production'
        ? 'https://mydomain.cdn.imgeng.in/'
        : '/',

    // For the default Vue CLI Webpack config,
    // @see https://github.com/vuejs/vue-cli/blob/dev/packages/%40vue/cli-service/lib/config/base.js#L19-L23
    chainWebpack: config => {
        config.module.rule('images')
            .use('url-loader')
            .tap(options => {
                // options.fallback is file-loader
                options.fallback.options.name += '[query]'
                return options
            })
    },
}

@alexander-akait
Copy link
Member

alexander-akait commented Feb 19, 2020

Maybe also (i am not familiar with Vue CLI):

 config.module.rule('images')
            .use('file-loader')
            .tap(options => {
                // options.fallback is file-loader
                options.name += '[query]'
                return options
            })

@kamermans
Copy link

Yes, that should also work, although I imagine it would replace the existing rule (url-loader) and revert to the default name (which is computed like this in Vue CLI). I'm not sure what impact that would have on the downstream loaders.

@alexander-akait
Copy link
Member

alexander-akait commented Feb 19, 2020

@kamermans solved, please try https://github.com/webpack-contrib/file-loader/releases/tag/v5.1.0, the simple example https://github.com/webpack-contrib/file-loader#cdn, feel free to leave feedback

@kamermans
Copy link

Awesome, thanks!

I've tested it in my site: https://distracted-ritchie-1f0417.netlify.com/

For some reason it works for the background image (via CSS) but not for the <img> tag, which has ended up with the image sources being replaced with [object Module]:

<img srcset="[object Module], [object Module] 1.5x, [object Module] 2x" src="[object Module]" alt="Vue logo" class="example">

This doesn't seem related to your change though, since it happens with file-loader@v5.0.0 as well. Vue CLI is shipping with "file-loader": "^4.2.0", so there must be something significant that changed in v5.

@alexander-akait
Copy link
Member

Yes, it is bug in html-loader, ETA next release is next week

@kamermans
Copy link

Hey, thanks again, I can confirm that everything is working properly (my issue above was actually resolved by disabling esModule).

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

Successfully merging a pull request may close this issue.

4 participants