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

fix: subresource integrity #803

Merged
merged 1 commit into from
Aug 7, 2021

Conversation

jarrett-confrey
Copy link
Contributor

Summary

this pull request solves the problem of integrity not being added to script tags when using webpack-subresource-integrity. it seems that either #436 did not include everything necessary or possibly my configuration is different but i was unable to get things working properly without these changes. other users are seeing this issue as well as seen in #563

Test plan

  • add webpack-subresource-integrity to your webpack plugins
  • you should now see the integrity attribute in loadable-stats.json and on your script tags when using ChunkExtractor.getScriptTags

@@ -123,6 +123,25 @@ class LoadablePlugin {
},
)
})

Copy link
Collaborator

Choose a reason for hiding this comment

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

checked - SriPlugin extends assets information at PROCESS_ASSETS_STAGE_OPTIMIZE_HASH stage, so at PROCESS_ASSETS_STAGE_REPORT all extra fields should be already a part of stats.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

youre right, thanks! i wasnt seeing the integrity available at this stage before but i must have been looking in the wrong place at the time

Copy link
Collaborator

@theKashey theKashey left a comment

Choose a reason for hiding this comment

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

There should be close to zero changes around LoadablePlugin.apply

@jarrett-confrey
Copy link
Contributor Author

There should be close to zero changes around LoadablePlugin.apply

done, thanks for the feedback!

Copy link
Collaborator

@theKashey theKashey left a comment

Choose a reason for hiding this comment

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

Wow it's really simple now! I'll review it shortly

@jarrett-confrey
Copy link
Contributor Author

hey @theKashey, just checking if youve had a chance to take another look? currently i wrote a new plugin to do this work for our project but i would prefer not to keep it around. thanks!

@theKashey theKashey merged commit 9b34195 into gregberge:main Aug 7, 2021
@theKashey
Copy link
Collaborator

Verified and merged. 👍

@jarrett-confrey
Copy link
Contributor Author

thank you @theKashey! any chance for a new release with this in 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.

None yet

2 participants