-
-
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
Move registerCoreWithSerializer to its own file #9396
Conversation
RequestGraph, | ||
// $FlowFixMe[unclear-type] | ||
}): Array<[string, Class<any>]>)) { | ||
if (ctorToName.has(ctor)) { |
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.
Was this function somehow getting called multiple times now? Looks like this wasn't there before
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.
Ah we had a different check for this before https://github.com/parcel-bundler/parcel/pull/9396/files#diff-9575fdb466517de49a2fcba34666f6aafe312d93c8c66978f8d7e6b27464b792L51-L55
and I thought I needed to change it bc the function is called in Parcel.js
and worker.js
and I was running into this error
parcel/packages/core/core/src/serializer.js
Lines 11 to 13 in 0ab3f0f
if (ctorToName.has(ctor)) { | |
throw new Error('Class already registered with serializer'); | |
} |
but the error was happening bc I had an unused import that was calling the function again. I'll change it back
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.
lgtm
↪️ Pull Request
This moves the
registerCoreWithSerializer
function to a different module to prevent circular dependency problems💻 Examples
🚨 Test instructions
✔️ PR Todo