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

WIP ported code to webpack 4 #56

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

malectro
Copy link

i went through this today for my team, but we don't use all the options so i'm not sure how to fully test without a suite. maybe an okay starting point?

@mojoaxel
Copy link

This fixes #55


return source.replace(
`"${chunkManifestFilename}"`,
`window['${this.options.manifestVariable}']['${chunkId}']`,
Copy link

Choose a reason for hiding this comment

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

Should chunkId be quoted here? I had to unquote it because I think it's referring to the name variable in scope. (I've never done Webpack hooks stuff, so I might be completely wrong here)

Copy link
Author

@malectro malectro Apr 18, 2018

Choose a reason for hiding this comment

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

yup, my bad.

edit: actually it looks like it's quoted on L 56 in the master so probably should be as is.

@bf
Copy link

bf commented Apr 27, 2018

@brudil can we please merge this so I can use this plugin with webpack 4?

@brudil
Copy link

brudil commented Apr 29, 2018

@bf I'm nothing to do with this repo, just a user of the plugin itself.

@niemyjski niemyjski mentioned this pull request Jun 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants