-
Notifications
You must be signed in to change notification settings - Fork 392
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
Changes from all commits
b33ce31
5c4720c
3052b41
0f34dd8
846c8cd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@clerk/clerk-js": patch | ||
--- | ||
|
||
Ensure we don't access `window.addEventListener()` and `window.dispatchEvent` in non-browser environments. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,15 +35,23 @@ class InjectedWeb3Providers { | |
coinbase: 'Coinbase Wallet', | ||
metamask: 'MetaMask', | ||
} as const; | ||
static #instance: InjectedWeb3Providers | null = null; | ||
|
||
constructor() { | ||
private constructor() { | ||
if (typeof window === 'undefined') { | ||
return; | ||
} | ||
window.addEventListener('eip6963:announceProvider', this.#onAnnouncement as EventListener); | ||
window.dispatchEvent(new Event('eip6963:requestProvider')); | ||
} | ||
|
||
public static getInstance(): InjectedWeb3Providers { | ||
if (!InjectedWeb3Providers.#instance) { | ||
InjectedWeb3Providers.#instance = new InjectedWeb3Providers(); | ||
} | ||
return InjectedWeb3Providers.#instance; | ||
} | ||
|
||
get = (provider: InjectedWeb3Provider) => { | ||
return this.#providers.find(p => p.info.name === this.#providerIdMap[provider])?.provider; | ||
}; | ||
|
@@ -56,4 +64,4 @@ class InjectedWeb3Providers { | |
}; | ||
} | ||
|
||
export const injectedWeb3Providers = new InjectedWeb3Providers(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exporting this makes it global (calling the |
||
export const getInjectedWeb3Providers = () => InjectedWeb3Providers.getInstance(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Odd that this didn't catch it. Does this mean expo is defining
window
, but notaddEventListener
? 😵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.
and looks like expo too