Skip to content
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

feat: support htmlwebpackplugin v4 #55

Merged
merged 2 commits into from
Jan 31, 2020
Merged

feat: support htmlwebpackplugin v4 #55

merged 2 commits into from
Jan 31, 2020

Conversation

Austaras
Copy link
Contributor

fix #30

@erezvish
Copy link

Oh wow, I see you got this already.
Thanks for making this fix!

@erezvish
Copy link

@DanielSchaffer
Who is supposed to review the changes, one of the current collaborators, you, or anyone interested?

@DanielSchaffer
Copy link
Owner

@erezvish - hi, I’m sorry for the lack of communication! I’m in the process of switching jobs, so I’ve been scrambling to wrap things up and haven’t had time to look. I’ll try to set aside some time today to take a look so you guys can get unblocked.

@DanielSchaffer
Copy link
Owner

DanielSchaffer commented Jan 25, 2020

@Austaras, @erezvish - my apologies for the delay - there were a ton of things that came up last minute while trying to leave my former coworkers in a reasonably caught up state, and I wasn't able to get to this until now.

What you've got so far looks great, but I must also apologize that I forgot that the HtmlWebpackPlugin hooks are also referenced in NormalizeModuleIdsPlugin - those will need to use the same logic you added to BabelMultiTargetHtmlUpdater. It might make sense to move your isV4 and hook logic into functions their own file.

I'd recommend trying your changes out by using the examples - you can start the dev server by running this from the command line:

NODE_OPTIONS="--max-old-space-size=8192" yarn start

Unfortunately, the server must be restarted when you make changes to the plugin code - I'm not sure there's even a way to do incremental builds for a Webpack plugin.

There are several examples, so the initial build may take a while to complete, especially on older machines - but once it's done, you can view the example projects by visiting http://localhost:3002/examples/angular/, and then following the links to each of the other examples. If all of the examples turn the background green after loading, you should be good to go.

Additionally, while I realize I haven't set up a formal "contributing guidelines", I'd much appreciate it if you could do the following:

  • use git rebase -i to squash your commits into logical chunks - multiple commits are fine if they do unrelated things, but I'd prefer to avoid sequential commits that are something to the effect of "do the thing", and then "that didn't work, actually do the thing"
  • please follow the guidelines established by Conventional Commits for your commit messages.

Thank you so much for your contributions on this, it is much appreciated!

EDIT: I've added CONTRIBUTING

fix package.json

make it work

fix normalize module id
@Austaras
Copy link
Contributor Author

Austaras commented Jan 28, 2020

Sorry for the dealy. Change as you requested. The Angular routing seems like broken in your branch too, other exmaple works fine

@Austaras
Copy link
Contributor Author

@DanielSchaffer would you take some time review my updated commit?

@DanielSchaffer
Copy link
Owner

DanielSchaffer commented Jan 31, 2020

@Austaras I believe the broken Angular example is just due to using the wrong typescript version .. I'll fix that in another commit

EDIT: There's a fix on develop if you want to merge or rebase on your branch

Copy link
Owner

@DanielSchaffer DanielSchaffer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The missing quotes on undefined is breaking it when used with html-webpack-plugin@^3 - make sure you run the examples with both versions to ensure it works correctly. If you've got a dist directory from building the plugin previously, you may need to run yarn build after installing a different version so the change gets picked up.

src/html-webpack-plugin.polyfill.ts Outdated Show resolved Hide resolved
Co-Authored-By: Daniel Schaffer <dan@dandoes.net>
@DanielSchaffer DanielSchaffer merged commit 13f09d6 into DanielSchaffer:develop Jan 31, 2020
@DanielSchaffer
Copy link
Owner

@Austaras @erezvish - this will be released as 2.5.0 (note: 2.4.0 is skipped due a some tagging/versioning snafu). Unfortunately, I'm having some issues with CI as Travis seems to have moved my cheese a bit - I can't poke at it right now, but I'll try and come back to it later this afternoon.

@erezvish
Copy link

Thank you both for this, it is much appreciated!
I'll try the new version when released, no worries though if integration takes a little time.
Thank you again

@DanielSchaffer
Copy link
Owner

DanielSchaffer commented Feb 1, 2020

@Austaras @erezvish pre-release: 2.5.0-next.1

Let me know if that works as expected for you and I'll cut the final 2.5.0 release. Thanks for your efforts and for bearing with me and my delays!

@Austaras
Copy link
Contributor Author

Austaras commented Feb 1, 2020

@DanielSchaffer Many thanks for your great plugin! I tried in my project and find few minor problems

  1. issue use single runtime chunk will cause error #56
  2. typing for babel-loader is not bundled so fork-ts-checker will compain about it

evrything else works fine

@erezvish
Copy link

erezvish commented Feb 2, 2020

Hi,
I can confirm that two builds are being created with Html webpack plugin (beta 8).
I'll need to work with it more to see if it's working properly.

Thanks for all the effort!

@DanielSchaffer
Copy link
Owner

@Austaras can you write up an issue for this with repro steps?

typing for babel-loader is not bundled so fork-ts-checker will complain about it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

support html-webpack-plugin@4
3 participants