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

Static Query string in Dynamic Imports Webpack4 #7778

Closed
skingston91 opened this issue Jul 24, 2018 · 2 comments
Closed

Static Query string in Dynamic Imports Webpack4 #7778

skingston91 opened this issue Jul 24, 2018 · 2 comments

Comments

@skingston91
Copy link
Contributor

skingston91 commented Jul 24, 2018

Bug report

What is the current behavior?

All of these imports work:

import responsiveImage from '/graphics/thumbnails/thumbnail-alien-attack.jpg?responsive'; // uses default loader params 

import responsiveImage from 'responsive-loader!/graphics/thumbnails/thumbnail-alien-attack.jpg?placeholder=true&placeholderSize=100&sizes[]=300,sizes[]=600;

return import(`responsive-loader/graphics/thumbnails/${name}.jpg`).then(
        ({ default: url }) => {
            return url;
        }

However anything requiring a query string in a dynamic name import returns the below error.

Dynamic Import and usage:


const getImage = (name: string) => {
	return import(`/graphics/thumbnails/${name}.jpg?responsive`).then(
		({ default: url }) => {
			return url;
		}
	);
};

Promise.all(
			coursesToShow.map(act => {
				return getImage(`thumbnail-${act.id}`).then(image => {
					return { ...act, image };
				});
			})
		).then(courses => this.setState({ courses }));


image

This has been bought up previously in the webpack stack overflow, the closet is below:
https://stackoverflow.com/questions/46652079/webpack3-query-string-in-dynamic-require-or-single-file-multiple-outputs from a year ago.

I understand Webpack is a static bundler and I believe this information is available at compile time. The queryString is used as part of our webpack config for detecting if we want to use responsive images or not and thusly which loader. As far as I can tell this is a bug because this is not making the image imports anymore dynamic.

module: {
		rules: [
			{
				test: imageRegex,
				oneOf: [
					{
						resourceQuery: /responsive/,
						loader: 'responsive-loader',
						options: {
							adapter: require('responsive-loader/sharp'),
							sizes: [300, 600, 1200, 2000],
							name: 'graphics/[name].ckh.[hash].[ext]',
							placeholder: true,
							placeholderSize: 100
						}
					},
					{
						use: [
							{
								loader: 'file-loader',
								options: {
									name: '[name].ckh.[hash].[ext]',
									outputPath: 'graphics/'
								}
							},
							{
								loader: 'image-webpack-loader',
								options: {
									bypassOnDebug: true
								}
							}
						]
					}
				]
			},

If the current behavior is a bug, please provide the steps to reproduce.
Dynamically Import a folder of images and add a query string as part of the promise.

What is the expected behavior?
Images with dynamic loading through webpack should be able to have static query strings passed into them or at least for usage internally inside webpack.

Other relevant information:

webpack version:
"webpack": "^4",

Node.js version:
"node": "8.11.3"

Operating System:
MacOs High Sierra
Additional tools:

Sorry if this intended behaviour and i'm wrong, but this seems like a bug to me. Let me know if you want a better example :) and thanks for the good work!

@skingston91 skingston91 changed the title Webpack4 query string in Dynamic Require Query string in Dynamic Require Webpack4 Jul 24, 2018
@skingston91 skingston91 changed the title Query string in Dynamic Require Webpack4 Static Query string in Dynamic Require Webpack4 Jul 24, 2018
@skingston91 skingston91 changed the title Static Query string in Dynamic Require Webpack4 Static Query string in Dynamic Imports Webpack4 Jul 24, 2018
@sokra
Copy link
Member

sokra commented Jul 25, 2018

This can be implemented. The underlying ContextModule does support that. Only the import() parser need to be fixed.

Send a PR

@skingston91
Copy link
Contributor Author

Okay Thanks :) I'll give it a go next week

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

No branches or pull requests

3 participants