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

Can't hook async behavior in processAssets #262

Closed
tomlagier opened this issue Apr 29, 2021 · 2 comments
Closed

Can't hook async behavior in processAssets #262

tomlagier opened this issue Apr 29, 2021 · 2 comments

Comments

@tomlagier
Copy link
Contributor

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.

stage: Infinity

  • 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
Copy link
Owner

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
Copy link

stale bot commented Jun 29, 2021

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.

@stale stale bot closed this as completed Jun 29, 2021
tomlagier added a commit to tomlagier/webpack-manifest-plugin that referenced this issue Aug 8, 2021
shellscape pushed a commit to tomlagier/webpack-manifest-plugin that referenced this issue Jan 11, 2022
shellscape added a commit that referenced this issue Jan 11, 2022
…263)

* feat: Added ability to adjust plugin processAssets hook stage (#262)

* test: update snapshots

Co-authored-by: shellscape <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants