-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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): Make sure to return a callback manager if configure hooks #7366
fix(core): Make sure to return a callback manager if configure hooks #7366
Conversation
…ike to use This preserves existing behavior except for when library creators want handlers registered with the `registerConfigureHook` function. In that case, we want to make sure we return a callback manager.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
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.
Small nit, can apply myself if you don't have time
@@ -1276,8 +1276,12 @@ export class CallbackManager | |||
handler = new (handlerClass as any)({}); | |||
} | |||
if (handler !== undefined) { | |||
if (!callbackManager?.handlers.some((h) => h.name === handler!.name)) { | |||
callbackManager?.addHandler(handler, inheritable); | |||
if (!callbackManager) { |
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.
Since this can potentially create a callback manager - let's move this whole block above where we add the inheritableTags
and inheritableMetadata
so that those are added properly
Set a campaign manager, if we know there are callback managers we'd like to use
This preserves existing behavior except for when library creators want handlers registered with the
registerConfigureHook
function. In that case, we want to make sure we return a callback manager.Twitter: @ibolmo