Usage of webpack public path inconsistent

[HoneyryderChuck]

Related to #5

It seems I might have jumped too early. There is an issue with the option useWebpackPublicPath and its handling in the HtmlWebpackPlugin. First, I'm of the opinion that this option is irrelevant (the manifest.json should always be generated in the output path). Nevertheless, I'd like to go through the inconsistency of its usage right now. So, with following webpack options:

• path: dist/
• publicPath: /assets/

as ./dist and with the (default) value of false, it will:

• generate the manifest.json in the output path: ./dist/manifest.[hash].json
• inject the link tag <link rel="manifest" href="manifest.[hash].json" />

so, the file is in the proper place, but the url is wrong (it should contain the publicPath from webpack). when I switch it to true (use webpack public path):

• generates the manifest.json in this path: ./dist/assets/manifest.[hash].json
• inject the link tag <link rel="manifest" href="/assets/manifest.[hash].json" />

so now the URL is right, but the file isn't in the proper place, therefore the server will return 404.

My feeling is that, somewhere in the codebase, both options are being misused. Nevertheless, I'd remove the useWebpackPublicPath option altogether and just use the right webpack variables.

What do you think?

[arthurbergmz]

Okay, let me see if I got this right.
If you mean we are going to have problems with websites that were deployed on multiple domains when useWebpackPublicPath is active, I guess you're right.

I think I share the same opinion as you, actually. Maybe not a complete removal, you know? If someone really needs this option, it will be there. Until a moment that shows a real deal using publicPath, useWebpackPublicPath is going to be false by default, not interfering in how WebpackPwaManifest works.

I'll keep this issue open for further discussion.

[HoneyryderChuck]

f you mean we are going to have problems with websites that were deployed on multiple domains ...

I don't think this is only for multiple domains. This has to do with mixing the URL path and the file system path. Webpack has other mechanisms to support multi-domain, and they're irrelevant for this issue.

Maybe not a complete removal, you know?

If you want to be full SemVer, I'd say you could deprecate it and later removed in a 1.0 . But I just wanted to stress the point that this option doesn't seem to have a point in existing.

My point being, besides the bug, I don't see a real use case where the option is decisive. But I might be missing smth.

[HoneyryderChuck]

I think the issue might be here, where you're only injecting the filename and omitting the output path.

[arthurbergmz]

It is actually here. publicPath is an empty string, if useWebpackPublicPath is off, or the actual publicPath.

I actually can't really see where the bug is. Everything was working fine when I tested it. Could you provide something I can run on my machine?

[HoneyryderChuck]

Could you provide something I can run on my machine?

Sure. This would be the most self-contained of the examples:

output: {
  path: path.resolve(__dirname, './dist/assets'),
  filename: '[name].bundle.js',
  publicPath: '/assets/',
  sourceMapFilename: '[name].map'
},

So, I'm bundling my assets to the ./dist/assets directory, and the urls are being build with prefix /assets/, so, a rando app.js entry point will be bundled into ./dist/assets/app.js, and will be injected in my template with href /assets/app.js.

I'm assuming I don't need to pass the HtmlWebpackPlugin and WebpackPwaManifest plugin setups, as they're fairly ordinary.

The bug can be reproduced in that the manifest.json will be built into ./dist/assets/manifest.[hash].json (good) and injected with href manifest.[hash].json(bad) with the default settings.

[arthurbergmz]

Oh, now I see!
You're right, manifest.json should also be generated in the output path, among the other assets.

I'm going to fix it right now.

[arthurbergmz]

Could you run a test on your project using my last commit? I hope it works as expected.

[HoneyryderChuck]

I've run this with the option in default mode (I've seen you deprecated it).

The URL seems to have been properly generated:

<link rel="manifest" href="/assets/manifest.5f278e4ec678db6c28522b39edca31c6.json" />

but the location where the file is still sadly the wrong one:

> ls dist/assets/assets/man*
dist/assets/assets/manifest.5f278e4ec678db6c28522b39edca31c6.json

[arthurbergmz]

I'm working on a definitive fix this dawn.

[arthurbergmz]

@HoneyryderChuck Hey, I think I managed a way of using public path the right way. Can you run another test using my newest commit, please? Thanks a bunch for helping me out with this bug!

[HoneyryderChuck]

@arthurbergmz file is in the right place, and the url is the right one. I approve this fix 👍

[arthurbergmz]

Fix released at v3.1.1. Please, update.

Thank you very much for all your support!

[HoneyryderChuck]

Thx also for the quick fix, mate! :)