-
-
Notifications
You must be signed in to change notification settings - Fork 20
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
Fix get images #51
base: master
Are you sure you want to change the base?
Fix get images #51
Conversation
…esult.compilation.assets['index.html'].source() no longer works
…ipts in the end of body, instead it puts the script in the end of head. I added the attr defer in the script tag because HTMLWebpackPlugin add that in the script tag now. I think that these changes do not affect the expected result .
…by webpack because result.compilation.assets['index.html'].source() and result.compilation.assets['other-page.html'].source() no longer works
…TMLWebpackPlugin no longer puts scripts in the end of body, instead it puts the script in the end of head. I added the attr defer in the script tag because HTMLWebpackPlugin add that in the script tag now. I think that these changes do not affect the expected result.
…esult.compilation.assets['index.html'].source() no longer works
… in the end of body, instead it puts the script in the end of head. I added the attr defer in the script tag because HTMLWebpackPlugin add that in the script tag now. I think that these changes do not affect the expected result.
…result.compilation.assets['index.html'].source() no longer works
…s scripts in the end of body, instead it puts the script in the end of head. I added the attr defer in the script tag because HTMLWebpackPlugin add that in the script tag now. I think that these changes do not affect the expected result.
…result.compilation.assets['index.html'].source() and result.compilation.assets['other-page.html'].source() no longer works
…template-filename-other.html because HTMLWebpackPlugin no longer puts scripts in the end of body, instead it puts the script in the end of head. I added the attr defer in the script tag because HTMLWebpackPlugin add that in the script tag now. I think that these changes do not affect the expected result.
…result.compilation.assets['index.html'].source() no longer works
…o longer puts scripts in the end of body, instead it puts the script in the end of head. I added the attr defer in the script tag because HTMLWebpackPlugin add that in the script tag now. I think that these changes do not affect the expected result.
…result.compilation.assets['index.html'].source() no longer works
… puts scripts in the end of body, instead it puts the script in the end of head. I added the attr defer in the script tag because HTMLWebpackPlugin add that in the script tag now. I think that these changes do not affect the expected result.
…pplied to the plugin and to adapt to the latest version of webpack and HTMLWebpack
…rsion of the webpack and HTMLWebpackPlugin
…result.compilation.assets['index.html'].source() no longer works
…rsion of the webpack and HTMLWebpackPlugin
…s in the end of body, instead it puts the script in the end of head. I added the attr defer in the script tag because HTMLWebpackPlugin add that in the script tag now. And also how I changed the documentation of the example.
…result.compilation.assets['index.html'].source() and result.compilation.assets['about.html'].source() no longer works
…se HTMLWebpackPlugin no longer puts scripts in the end of body, instead it puts the script in the end of head. I added the attr defer in the script tag because HTMLWebpackPlugin add that in the script tag now. I think that these changes do not affect the expected result.
…result.compilation.assets['index.html'].source() and result.compilation.assets['about.html'].source() no longer works
…ripts in the end of body, instead it puts the script in the end of head.\nI added the attr defer in the script tag because HTMLWebpackPlugin add that in the script tag now.\nI think that these changes do not affect the expected result.
…result.compilation.assets['index.html'].source() and result.compilation.assets['about.html'].source() no longer works
…ts scripts in the end of body, instead it puts the script in the end of head.\nI added the attr defer in the script tag because HTMLWebpackPlugin add that in the script tag now.\nI think that these changes do not affect the expected result.
…result.compilation.assets['index.html'].source() and result.compilation.assets['about.html'].source() no longer works
…result.compilation.assets['index.html'].source() and result.compilation.assets['about.html'].source() no longer works
…ipts in the end of body, instead it puts the script in the end of head. I added the attr defer in the script tag because HTMLWebpackPlugin add that in the script tag now. I think that these changes do not affect the expected result.
The time was increased in the tests due to the fact that sometimes mocha gives false consumption of time.
thanks so much for putting this together! it'll take some time for me to review (and as you could tell time to get to it in the first place) but definitely interested in these changes it looks like the tests failed due to the node version: https://github.com/colbyfayock/html-webpack-partials-plugin/runs/5694847186?check_suite_focus=true i currently have the nvmrc pointed at v10, where im not opposed to bumping it up: https://github.com/colbyfayock/html-webpack-partials-plugin/blob/master/.nvmrc it's been a while since i've worked directly in webpack, do you have any thoughts on changing the minimum required node version for this package? looks like it needs to be >12.13.0 would you mind bumping the version in .nvmrc based on that? |
Hi! Yes, of course. |
It didn't really need to be static. The reason for the change was because it was causing an error in the Page Template test, I will try to explain how it caused that error: The filesProcessed variable saves the name of the files processed by HTMLWebpackPlugin and is used later to inject the partial, this occurs when in the configurations template_filename is equal to *, because the variable was static it was causing an error in the Page test Template, when mocha went through that test, the filesProcessed variable had saved the name of the files processed by HTMLWebpackPlugin from the tests prior to this test, when doing the iteration of the variable to obtain each file processed by HTMLWebpackPlugin and when trying to obtain the file, it could not be found because it belonged to another test causing the Page Template test to fail.
…e Page Template test, probably due to changes I made to the plugin.
…odules need the nodejs version 12.13.0
I changed the version in .nvmrc and I changed the version of nodejs used in the workflow test, I think it was not necessary change the version in .nvmrc, with act https://github.com/nektos/act the test passed with nodejs 10 or 12 in .nvmrc but "act" failed when in /.github/workflows/tests.yml nodejs had 10 version. Also, I made a change in a variable that I had added previously and which caused an error in the mocha tests and I added an timeout in the Analytics test. |
hey @sgb004 sorry that i'm only now responding to this. this project has taken a pretty low priority for me that said I'm not comfortable with the changeset here. especailly given the amount of time im spending on this, i need to be extra cautious that something introduced isn't going to create issues for existing users of the plugin seeing the output differences don't make me confident that things aren't broken, particularly:
that said i'm not really sure what the path for this looks like moving forward if you're still interested in working on this after all this time. i appreciate the contribution and apologize for how long it took to get back |
I was looking for a plugin that could easily reuse an HTML fragment in webpack and I was very pleasantly surprised to see that HTMLWebpackPartialsPlugin could do what I was looking for and in such a simple and easy to use way.
However, I found a small problem, the plugin did not process the imgs tags, when I searched for an answer I found that some people also had the same problem #7 So I tried to take a look at the code, after reviewing the plugin code, I came to the conclusion that it did not have a way to process the imgs tags so that later webpack could extract the images, I imagine that initially webpack, HTMLWebpackPlugin or html-loader were getting the images, but I guess with the latest updates this was no longer the case. So I tried to correct this little problem.
With this pull-request my intention is that the plugin can process the imgs tags and that webpack, HTMLWebpackPlugin or html-loader can process the images.
So that the plugin could process the imgs tags, what I did was create a new instance of HTMLWebpackPlugin and let HTMLWebpackPlugin process the partial, then through the processAssets hook with the stage PROCESS_ASSETS_STAGE_SUMMARIZE obtain the html of the partial already processed to finally inject it into the template or templates.
I replaced the current PROCESS_ASSETS_STAGE_SUMMARIZE hook of HTMLWebpackPlugin with 3 hooks:
All of the above I modified in index.js
To achieve these changes I also modified partial.js, where I added a unique_name with which to identify the file and thus be able to obtain the html and later delete it. In this file I also modified the createTemplate method where it now receives the html and converts it to a loash template instead of getting it directly from the source file like it currently is.
It seems to me that this is a crude way of processing the partial, I made another version where I managed to obtain the code that processes the file in HTMLWebpackPlugin, this with the aim of trying to improve performance and not load the entire core of HTMLWebpackPlugin, however in the tests the times remained almost the same. You can check the alt version in https://github.com/sgb004/html-webpack-partials-plugin/tree/fix_get_imgs_alt_ver
Other changes I made was updating the dependencies, I added uuid and changed the mocha processing time, I had to update the tests since webpack no longer returns the source, I changed the code of the tests so that instead of getting result.compilation.assets[‘file.html'].source() the html is obtained using fs.
Updating webpack and HTMLWebpackPlugin caused the html of the examples to change, so I had to change the html of test/fixtures, really the only change I made was to move the script from the body to the header since HTMLWebpackPlugin puts it in the header, I wanted to keep the test/fixtures files as they were, but that would involve changing the settings in each of the examples and I preferred to keep the examples simple.