jharris4 / html-webpack-tags-plugin

lets you define html tags to inject with html-webpack-plugin
MIT License
255 stars 31 forks source link

add raw source support and resolve file path #33

Closed dushaobindoudou closed 6 years ago

dushaobindoudou commented 6 years ago

add raw source support and resolve file path

jharris4 commented 6 years ago

Hi! Thanks for the PR, this looks like a useful feature!

Do you think you could take a minute to add a unit test for this new feature though?

Adding unit tests helps both make sure this code works correctly, and also gives a kind of documentation for how this feature works.

jharris4 commented 6 years ago

I removed the raw source feature since it didn't seem to be doing anything.

But I did add a new plugin option so you can do { resolvePaths: true } if you want your assets paths to be resolved to absolute paths.

This was released in version 1.0.5

I'd be happy to look into the assets.rawData = ... code you added again if you can explain why you added it.

dushaobindoudou commented 6 years ago

sometimes we need to add some inline code to the html so i add the rawData field

jharris4 commented 6 years ago

Ok but how does it work? I couldn’t find rawData mentioned anywhere else...

dushaobindoudou commented 6 years ago

we use custom templates.

`<% for (var css in htmlWebpackPlugin.files.css) { %> <% if(htmlWebpackPlugin.files.rawData[htmlWebpackPlugin.files.css[css].substr(htmlWebpackPlugin.files.publicPath.length)]) {%>

        <% } %>
    <% } %>`

this can work

jharris4 commented 6 years ago

Ok, I see what you're getting at. It made me wonder if html-webpack-plugin supports inline content directly and a quick search found this:

https://github.com/jantimon/html-webpack-plugin/issues/104

I'm curious if any of the solutions listed there would also work for you?

dushaobindoudou commented 6 years ago

We need to add additional asset resources, not all cases need inline, in most cases still use src / href way, so Improved your plugin。I think this plugin supports raw is not a bad thing, the plugin can support more scenarios

jharris4 commented 6 years ago

Ok, but new features need documentation and tests! 😉

I’m trying to make sure I understand the feature you added enough to add those for it...

dushaobindoudou commented 6 years ago

I will resubmit a pull request, including tests and documentation