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(core): call onAfterBuild on every build #1771

Merged
merged 1 commit into from
Mar 9, 2024

Conversation

xc2
Copy link
Contributor

@xc2 xc2 commented Mar 8, 2024

Summary

Related Links

fixes #1765

Checklist

  • Tests updated.
  • Documentation updated.

Copy link

netlify bot commented Mar 8, 2024

Deploy Preview for rsbuild ready!

Name Link
🔨 Latest commit 7c77c4c
🔍 Latest deploy log https://app.netlify.com/sites/rsbuild/deploys/65eb4b457d2fef0008849f13
😎 Deploy Preview https://deploy-preview-1771--rsbuild.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@xc2 xc2 force-pushed the fix/onAfterBuild-on-every-build branch from 716b750 to 7c77c4c Compare March 8, 2024 17:30
@xc2 xc2 marked this pull request as ready for review March 8, 2024 17:44
@chenjiahan chenjiahan merged commit c168c21 into web-infra-dev:main Mar 9, 2024
10 checks passed
@chenjiahan
Copy link
Member

Nice 😄

@xc2 xc2 deleted the fix/onAfterBuild-on-every-build branch March 9, 2024 02:07
await p;
};

try {
Copy link
Member

Choose a reason for hiding this comment

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

@xc2 I wonder when will hooks.done.tapPromise throw an error here...

Copy link
Member

Choose a reason for hiding this comment

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

ok I get it, the hooks.done is a sync hook when using multiCompiler..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

MultiCompiler's done hook is a SyncHook. https://github.com/webpack/webpack/blob/6842c9a2c9b70e760143188ffa5baa19a72799cb/lib/MultiCompiler.js#L63

SyncHook's tapPromise is a function that only throws

Copy link
Member

Choose a reason for hiding this comment

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

yeah, this means if onAfterBuild is an async function, it may not work as expected when Rsbuild use MultiCompiler (has multiple targets)

@chenjiahan chenjiahan mentioned this pull request Mar 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants