-
Notifications
You must be signed in to change notification settings - Fork 70
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
fix(core): call onAfterBuild
on every build
#1771
Conversation
✅ Deploy Preview for rsbuild ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
716b750
to
7c77c4c
Compare
Nice 😄 |
await p; | ||
}; | ||
|
||
try { |
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
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..
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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)
Summary
Related Links
fixes #1765
Checklist