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

feat(template): support custom template params #830

Closed
wants to merge 1 commit into from

Conversation

yyx990803
Copy link
Contributor

This allows the user to inject custom variables to be used in the template interpolation, e.g. simplifying webpackConfig.output.publicPath to something shorter.

Docs/tests are not included, but if this sounds like a good idea I can add those upon request.

@jantimon
Copy link
Owner

jantimon commented Jan 3, 2018

Right now this is already possible by accessing htmlWebpackPlugin.options.foo

@yyx990803
Copy link
Contributor Author

yyx990803 commented Jan 3, 2018

Yes, however we are building a tooling layer using html-webpack-plugin internally, and would like to expose to the users some variables that are easier to access (and less confusing), especially when the variable may be used many times in the template. foo vs htmlWebpackPlugin.options.foo can make quite some difference in this case.

e.g. in create-react-app, an extra pass of template interpolation was needed in order to expose PUBLIC_URL: https://github.com/facebookincubator/create-react-app/blob/master/packages/react-scripts/config/webpack.config.dev.js#L239-L243

We have similar needs in vue-cli.

@ezzatron
Copy link

I would also appreciate the ability to do this without using the htmlWebpackPlugin.options. prefix. I have written a Webpack plugin that integrates with html-webpack-plugin to provide a version string as a template variable. It would be nice to be able to simply use {{version}} in templates.

Another thing I'm using {{htmlWebpackPlugin.options.foo}} for is content where I have to generate multiple versions of a file for different brands/themes. But to avoid collisions with the actual plugin options, I feel the need to have a special key under which all my custom variables live. This means to access {{foo}}, it's more like {{htmlWebpackPlugin.options.variables.foo}}, which is starting to get a bit ridiculous.

It also seems like a code smell that my template has to know about the fact that I'm using Webpack at all, let alone which plugins I'm using.

@jantimon
Copy link
Owner

The original intend of the long name was to be explicit.

New devs in an existing project should be able to find their way to the variable origin.
Is there any reason for not using the create react plugin for your case?

@yyx990803
Copy link
Contributor Author

@jantimon I am using html-webpack-plugin in vue-cli and in our case, we intend to shield the user from the lower level details of html-webpack-plugin and expose something that's easier to understand.

create-react-app is introducing an extra, different interpolation syntax into the template, which is not optimal.

@jantimon
Copy link
Owner

If we would merge it this would mean that any addition to the template parameters might conflict with someone’s configuration.
What about adding an explicit config variable which would overwrite the default template variable instead of extending it?

@yyx990803
Copy link
Contributor Author

I understand the concern - but I think future additions should be nested under htmlWebpackPlugin anyway, right? It seems quite unlikely that we ever need to add more top level params. For the existing 4 top level params, we can check for conflict and warn the user that they are reserved.

Not pushing anything, but I'd love to be able to expose something as concise as possible for the end user.

@jantimon
Copy link
Owner

I would love to get rid of the getStats call for performance reasons.
What about the following?

Your proposed new templateParams could be empty, an object or a function.

If it’s empty it would be the same like today (just without the stats)
If it’s a function the current behavior with get stats could be reproduced manually.
If it’s an object it contains only the values of that given object.

@yyx990803
Copy link
Contributor Author

I like that, sounds very flexible.

@jantimon
Copy link
Owner

@yyx990803 @eezatron I prepared a new feature in #904.
Could you please let me know if this would solve this issue?

@lock
Copy link

lock bot commented May 31, 2018

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators May 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants