These changes allow empty public path in webpack.config.js #264
Conversation
|
@weaverryan I think I'll need some help with the functional test... |
|
It seems that we have some problems using javascript |
There was a problem hiding this comment.
Hey @stoccc!
Thanks for starting this! Ok, about the require.ensure call... this is Webpack's responsibility to resolve this path correctly: it does not read from our manifest.json. file. Actually, it basically just uses the final publicPath config key to figure out the path to any code split files. So... this is a real problem: I'm not sure if relative paths can work with code-splitting. Since Webpack uses publicPath to find these files, if you use a relative path, then it will try to request a different file based on the URL of your page - e.g. http://example.com/foo versus http://example.com/foo/bar.
I found this with some quick research: https://stackoverflow.com/questions/36675286/webpack-code-splitting-relative-url
If this is correct, it confirms my believe.... because the solution they're suggesting is to override the publicPath at runtime... which is the only time we know the true path.
Can you research this a bit more? It may be that relative paths will simply not work with code splitting (unless the user does this workaround at runtime (more on this: https://webpack.js.org/guides/public-path/#on-the-fly).
| // guarantee a single trailing slash | ||
| publicPath = publicPath.replace(/\/$/, ''); | ||
| publicPath = publicPath + '/'; | ||
| } |
There was a problem hiding this comment.
Does it make sense to move this inside of the first if statement? I mean: if the path is not absolute (https://....) or starting with a slash (/build), then we should do this? Or is there something special about an empty publicPath?
| @@ -227,6 +227,35 @@ describe('Functional tests using webpack', function() { | |||
| }); | |||
| }); | |||
|
|
|||
| it('Deploying to an unknown (at compile-time) subdirectory is no problem', (done) => { | |||
| const config = createWebpackConfig('public/build', 'dev'); | |||
| config.addEntry('main', './js/code_splitting'); | |||
There was a problem hiding this comment.
I think we should also include an entry that loads a CSS file normally (i.e. without code splitting) that includes a path to an image or a font. Then, after running webpack, we can manually check the contents of the final CSS file to make sure that the path to that image is correct. As I understand it, THAT is the biggest problem we're having with relative paths: the paths to images/fonts in CSS files was being generated incorrectly.
Unfortunately, the assertResourcesLoadedCorrectly method you're using below works for JS only - that's why we need to test the contents of the file.
This allows empty publicPath in webpack.config.js
Provided a functional test.
See #26 (comment)
I'm new to webpack/encore/nodejs/git, so I hope I didn't make any mistake :)