Can't hook async behavior in processAssets

tomlagier commented 3 years ago

First off, love this plugin. It's a foundational piece of Webpack and enables so much good work so thank you 🙏

I'm building a plugin that takes the manifest and generates some PHP files based on it. It'd like to zip up the PHP files into an archive after I finish writing them. Most zip libraries are async only. There are two aspects of WebpackManifestPlugin that make it impossible (as far as I can tell) to achieve this.

The plugin hooks at processAssets stage Infinity, which makes it impossible to register a hook for a later processAssets stage.

https://github.com/shellscape/webpack-manifest-plugin/blob/da910dcfc3984ef699e417a3ee9cb8bb7c35b890/lib/index.js#L50

The compiler hooks exposed by the plugin are sync, so I can't perform async work in them.

https://github.com/shellscape/webpack-manifest-plugin/blob/master/lib/hooks.js#L17

Manifest Plugin Version: 3.0.4

Expected Behavior / Situation

I can add new files asynchronously to the compilation using the output of the manifest.

Actual Behavior / Situation

I can't (as far as I can tell) add new files asynchronously to the compilation using the output of the manifest.

Modification Proposal

I think the simplest thing to do is just change the stage: Infinity to stage: webpack.Compilation.PROCESS_ASSETS_STAGE_ANALYSE. The description of that stage is: Analyse existing assets.

That seems to fit well within the scope of the plugin. Then I could hook PROCESS_ASSETS_STAGE_REPORT and ensure that my plugin runs after the manifest is created.

You could also argue that the behavior fits within PROCESS_ASSETS_STAGE_REPORT, which also seems like a good fit. This wouldn't preclude me from adding my own behavior as I could then make up a higher stage and use that, it's just a little less clean for me.

An alternative would be to change the SyncWaterfallHooks to AsyncSeriesWaterfallHooks. I don't like this as much because it would mean changing the emit helper function to async, which would have a larger footprint.

Let me know if there's an appetite for this change and I'm happy to submit a PR and update any tests.

shellscape commented 3 years ago

I think the simplest thing to do is just change the stage: Infinity to stage: webpack.Compilation.PROCESS_ASSETS_STAGE_ANALYSE. The description of that stage is: Analyse existing assets.

That's likely going to create a breaking change for many a user. While I'm open to this, it'll definitely have to be an option that modifies that stage, rather than asserting it for all users.

An alternative would be to change the SyncWaterfallHooks to AsyncSeriesWaterfallHooks. I don't like this as much because it would mean changing the emit helper function to async, which would have a larger footprint.

Same would apply here - it would have to be conditional and explicit, but I'm open to it.

stale[bot] commented 3 years ago

Hey folks. This issue hasn't received any traction for 60 days, so we're going to close this for housekeeping. If this is still an ongoing issue, please do consider contributing a Pull Request to resolve it. Further discussion is always welcome even with the issue closed. If anything actionable is posted in the comments, we'll consider reopening it. ⓘ

shellscape / webpack-manifest-plugin