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
Push manifest does not adhere custom changes in preact config file #1675
Comments
This issue also affects |
I'd lean towards calling this behavior correct, honestly. If you begin to alter expected file names through the config, there's no way for us to be aware of this, so you more or less become responsible for that change -- this means fixing locations in which our expectations will no longer be true. Altering behavior here would also likely be a breaking change. |
Hello @rschristian, thanks for your fast reply!
Well, I would not say so. In this case, it should not generate
I'm aware that this issue is "tax" for opinionated approach. So let me ask, what is the preferred way in
Thanks. |
Sorry, misspoke there. Definitely agree, we should fix that to output nothing (and alter assumptions about a
There is no preferred way. If you really need to make those changes, you'll likely need to make a fair number adjustments to the config. Is there any particular reason you want to make these changes? File servers should be more than capable of serving any file type from any location. You shouldn't need to segregate them out. |
Hello @rschristian,
In Europe, court already fined some company for not self-hosting fonts (while using Google Fonts CDN instead), because it led to leaking user IP addresses (in EU is IP address alone considered as PII) without user consents out of EU. So now we are required to self-host all fonts - or to be precise, all static files ourselves to be safe. If project uses few fonts, each with few unicode ranges (latin, latin-ext, etc.), few font variants (weights), it will end up in tens - even hundreds of font files in project root. Therefore I want them in separate directory, so I can navigate without problems (actually I don't need to touch fonts ever again, so there is no reason for them to pollute project root). When I was doing this, I distributed all files into my preferred directory structure (like in my previous comment). I wouldn't say, it is technical issue, just I don't like a big mess of files in project root. Thanks. |
So that context helps, a lot. If you're willing to forgo So // preact.config.js
export default (config, env, helpers) => {
const copyPlugin = helpers.getPluginsByName(config, 'CopyPlugin')[0];
if (copyPlugin) {
const { plugin } = copyPlugin;
// Looks a bit funny, but there's an implicit "<src input>" for "from", and "<build output>" for "to"
plugin.patterns.push({ from: 'fonts', to: 'fonts' }); // etc.
}
} Now, scripts and styles are a lot more involved as they are used for prerendering, file preloading, etc. These end up being a lot more "hard-coded" basically by necessity. Static assets though? Copying them to the output is no problem. |
Hello @rschristian, I'm familiar with
Checked source file (lines 16 and 18), responsible for this issue and I want to ask, if just simple Thanks. |
All that would need to change is the Let's say a user has a plugin that outputs additional assets, including a |
Hello @rschristian,
Of course, removing caret would solve this with even less effort, but IMO
Well as I said before, I don't understand underlying logic enough, but first things on my mind are, not using generic name as Thanks. |
More CLI flags = bad, especially for something like file names, which really shouldn't need to be altered. More options make it harder for new users to learn and more for us to test and maintain.
I'm extremely reluctant to add hacky fixes for self-inflicted problems.
If it's not clear, using your You're taking our config, cutting out parts you don't agree with, but not altering the dependants downstream. You absolutely could replace our manifest generating plugin with your own, which would realistically be the recommendation.
As far as I know, Webpack does not expose output file/chunkname to plugins, which is why this was built as it was. I'll look into it further, certainly could've been added over the years. Regardless, as this is just a cosmetic issue, and in your build output no less, I think my advice has to be to just skip editing file names if you don't absolutely need it. |
Hello @rschristian, got it! You can close this issue and mark it as Thanks. |
Sorry for the long-time-no-comment. Looks like webpack-manifest-plugin handles this quite well (to my surprise, quick glance at its docs don't do it justice). Fairly minimal too, so I at least have no problem adding it. Should be able to handle this case just fine. Will try to get a PR together tomorrow. Edit: #1680, when released, should correct both of your issues ( |
What is the current behaviour?
When I change output paths via
preact.config.js
file, generatedpush-manifest.json
file is wrong.Steps to reproduce:
preact.config.js
file:push-manifest.json
file:What is the expected behaviour?
Generate correct
push-manifest.json
file.Please mention any other relevant information:
preact-cli/packages/cli/lib/lib/webpack/create-load-manifest.js
Line 9 in aea6fed
In
filename
variable are not basenames of assets, but whole path, therefore regular expressions won't match.preact-cli/packages/cli/lib/lib/webpack/create-load-manifest.js
Line 16 in aea6fed
preact-cli/packages/cli/lib/lib/webpack/create-load-manifest.js
Line 18 in aea6fed
Please paste the results of
npx preact-cli info
here.The text was updated successfully, but these errors were encountered: