unbroken-dome / indexhtml-webpack-plugin

Index HTML plugin for Webpack
MIT License
42 stars 5 forks source link

"undefined" in some paths; ignores publicPath #4

Closed candrews closed 9 years ago

candrews commented 9 years ago

Here's a repository demonstrating the issue: https://github.com/candrews/indexhtml-webpack-plugin-bug

To see the problem, checkout the repository then run: ./webpack

Open dist/index.html You'll see this html:

<!DOCTYPE html>
<html>
<head>
<title>Site Template - Welcome!</title>
    <script src="app-f7131e69ff913f0468c5.js"></script>
    <link href="2109ced2c9e625698f1ae9b2fe55d03c.css">
</head>

<body>
        <img src="undefinedapp-9ce8fd458118f22141a23306ad57e881.png">
        <div class="Container">
                <div class="Header"></div>

                <div class="Menu">
                        <ul id="nav">
                                <li>menu item</li>
                                <li>menu item</li>
                                <li>menu item</li>
                                <li>menu item</li>
                                <li>menu item</li>
                                <li>menu item</li>
                        </ul>
                </div>

                <div class="Body"></div>
        </div>

</body>
</html>

There are 2 problems (which I think are both caused by the same issue):

  1. app-f7131e69ff913f0468c5.js should be /app-f7131e69ff913f0468c5.js
  2. undefinedapp-9ce8fd458118f22141a23306ad57e881.png should be /app-9ce8fd458118f22141a23306ad57e881.png

I think the problem is that indexhtml isn't setting __webpack_public_path__ when evaluating the JS-ified HTML. I'm not sure if __webpack_public_path__ is the right variable to set (I'm not familiar enough with Webpack internals), but I do know it's value should come from webpack.config.js's options.publicPath setting.

tkrullmann commented 9 years ago

Yes, the __webpack_public_path__ is indeed what causes the undefined in the output. I hadn't tried it with images before, but the url-loader seems a lot nicer to work with than the extract-text-plugin (which just removes the whole contents of the original source without even a hint where it's been extracted to).

It seems easy enough to add a

var __webpack_public_path__ = compilation.options.output.publicPath;

before eval'ing the HTML. I have to check what happens if {{publicPath}} is not specified in the options, or what a reasonable default value would be.

Also, the documentation states that the public path could be set at runtime, which would make it very hard to get the correct link.

candrews commented 9 years ago

I think, for the purposes of this plugin, the only solution is to say that setting publicPath at runtime isn't supported. There's just no way to implement dynamic publicPath using static html.

candrews commented 9 years ago

@tkrullmann this change works great for the img use case. But script src's are still wrong.

app-f7131e69ff913f0468c5.js should be /app-f7131e69ff913f0468c5.js

Can you address that too, please?

candrews commented 9 years ago

I think I figured out how to fix it: https://github.com/unbroken-dome/indexhtml-webpack-plugin/pull/5

This change fixes the problem in my testing.

tkrullmann commented 9 years ago

That works, thank you. I published v0.1.8 that contains the fixes.

candrews commented 9 years ago

So close... turns out the link href is still wrong. It is: 2109ced2c9e625698f1ae9b2fe55d03c.css but it should be: /2109ced2c9e625698f1ae9b2fe55d03c.css

candrews commented 9 years ago

Here's a PR solving that issue: https://github.com/unbroken-dome/indexhtml-webpack-plugin/pull/6

All the references look right now in the HTML. So I hope we've finally this fixed.

candrews commented 9 years ago

@tkrullmann can you please release version v0.1.9?

tkrullmann commented 9 years ago

done