search

waysact / webpack-subresource-integrity

Webpack plugin for enabling Subresource Integrity.
MIT License
357 stars 46 forks source link

Is the asset processing stage correct for this plugin? #168

Closed iiroj closed 2 years ago

iiroj commented 2 years ago

Hello,

I'm looking to integrate this plugin into my workflow, but noticed it's using the processAssets hook at the PROCESS_ASSETS_STAGE_OPTIMIZE_INLINE stage.

According to the docs this stage is used to

optimize the numbers of existing assets, e.g. by inlining assets into other assets.

Would the PROCESS_ASSETS_STAGE_OPTIMIZE_HASH stage be a better match, since it's used to

optimize the hashes of the assets, e.g. by generating real hashes of the asset content.

Thank you for this plugin!

jscheid commented 2 years ago

IIRC @sokra advised to use that stage. I can't find the discussion now, it might have taken place on Slack. This is perhaps relevant.

There's probably more to be said about it, but first-- what problem are you trying to solve?

jscheid commented 2 years ago

Also, have you tried running the test suite after changing the stage? I haven't recently but I bet it breaks.

iiroj commented 2 years ago

I was able to resolve my problem by ensuring the order of plugins is such that webpack-subresource-integrity is the last.

I just think that because plugins in this stage can still edit the contents of assets, it would be conceptually more correct to have this plugin in the next stage where assets should already be finalised.

jscheid commented 2 years ago

I was able to resolve my problem by ensuring the order of plugins is such that webpack-subresource-integrity is the last.

If you have a test case where this plugin breaks when it's in a different order, I'd appreciate if you could share it. There have been reports in the past (e.g. #79) but I've never been able to reproduce it.

I just think that because plugins in this stage can still edit the contents of assets, it would be conceptually more correct to have this plugin in the next stage where assets should already be finalised.

This plugin does modify assets -- when a chunk lazy-loads other chunks, it embeds the hashes of dependent chunks inside it -- so it's not as straightforward as it might look at first glance.

iiroj commented 2 years ago

Thanks for your reply. As per your explanation I now believe it is indeed in the correct stage already, but I might as well fork and try creating a test case for the plugin order anyway.

I'll close the issue for now.

jscheid commented 2 years ago

No problem @iiroj, thanks for looking into that test case. Let me know if you run into any troubles writing it, I think you're the first to try since the 5.0 release.

iiroj commented 2 years ago

@jscheid I opened a PR with a test demonstrating the issue.