-
Notifications
You must be signed in to change notification settings - Fork 394
fix(clerk-js): Ensure we do not access addEventListener
and dispatchEvent
in non-browser environments
#4095
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(clerk-js): Ensure we do not access addEventListener
and dispatchEvent
in non-browser environments
#4095
Conversation
…on-browser environments
🦋 Changeset detectedLatest commit: 846c8cd The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
}; | ||
} | ||
|
||
export const injectedWeb3Providers = new InjectedWeb3Providers(); |
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.
Exporting this makes it global (calling the addEventListener
and dispatchEvent
methods) and errors out in non-browser environments
addEventListener
and dispatchEvent
in non-browser environments
} | ||
|
||
export const injectedWeb3Providers = new InjectedWeb3Providers(); | ||
export const getInjectedWeb3Providers = () => InjectedWeb3Providers.getInstance(); |
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.
Exporting it now as a function but making sure it's still instantiated once
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.
👏
|
||
constructor() { | ||
private constructor() { | ||
if (typeof window === 'undefined') { |
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.
Odd that this didn't catch it. Does this mean expo is defining window
, but not addEventListener
? 😵
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.
React Native is defining window
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.
Description
This PR makes sure to NOT call browser specific methods (
addEventListener
,dispatchEvent
) implemented in 4059 in non-browser environments. Even though we are trying to block calls by checkingwindow
object, it'll still pass because React Native does have some polyfills to thewindow
object.Fixes SDK-1922
Checklist
npm test
runs as expected.npm run build
runs as expected.Type of change