Skip to content
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.

Sign up for GitHub

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

Update window.ethereum type to unknown #432

Merged
merged 4 commits into from
Mar 25, 2022
Merged

Update window.ethereum type to unknown #432

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
277ba78
Update window.ethereum type to any
erin-at-work Mar 25, 2022
88d0df9
Updates to 'unknown'
erin-at-work Mar 25, 2022
77adfa0
Add comment
erin-at-work Mar 25, 2022
6071cf2
Use JSDOC comments for deprecated api
erin-at-work Mar 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,23 @@ declare global {
interface Window {
CoinbaseWalletSDK: typeof CoinbaseWalletSDK;
CoinbaseWalletProvider: typeof CoinbaseWalletProvider;
ethereum?: CoinbaseWalletProvider;
/**
* For CoinbaseWalletSDK, window.ethereum is `CoinbaseWalletProvider`
*/
ethereum?: unknown;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i wonder if we could instead declare unknown | PreciseType so that type hinting is more useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had first attempted that, but it doesn't seem to get picked up.

// In a union an unknown absorbs everything

from microsoft/TypeScript#24439

image

and coinbaseWalletExtension:
image

I'll add back a comment

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah ok, oof then.

Copy link
Contributor Author

@erin-at-work erin-at-work Mar 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ljharb Thoughts on addressing this issue? #343

All declarations of 'ethereum' must have identical modifiers

In order to fix the above error, the type would need to be updated to:

interface Window {
  ethereum: any | undefined;
}

But I hesitate to make changes based on a library's conflicting type defs

coinbaseWalletExtension?: CoinbaseWalletProvider;

// deprecated
/**
* @deprecated Legacy API
*/
WalletLink: typeof CoinbaseWalletSDK;
/**
* @deprecated Legacy API
*/
WalletLinkProvider: typeof CoinbaseWalletProvider;
/**
* @deprecated Legacy API
*/
walletLinkExtension?: CoinbaseWalletProvider;
}
}
Expand All @@ -26,7 +37,12 @@ if (typeof window !== "undefined") {
window.CoinbaseWalletSDK = CoinbaseWalletSDK;
window.CoinbaseWalletProvider = CoinbaseWalletProvider;

// deprecated
/**
* @deprecated Use `window.CoinbaseWalletSDK`
*/
window.WalletLink = CoinbaseWalletSDK;
/**
* @deprecated Use `window.CoinbaseWalletProvider`
*/
window.WalletLinkProvider = CoinbaseWalletProvider;
}