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.

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

Make chainId, networkVersion, selectedAddress readonly and log deprecation warning #280

Merged
merged 5 commits into from
Sep 11, 2023

Conversation

jiexi
Copy link
Contributor

@jiexi jiexi commented Sep 8, 2023

Currently, it's possible to access the properties chainId, networkVersion, and selectedAddress on the provider object. These are not part of the EIP-1193 spec. More specifically, we are trying to remove networkId where possible and networkVersion currently poses an issue as it references networkId and that its removal may break some dapps if they are accessing this non-documented property. The plan is to add a deprecation warning when accessing any of these properties, and to fully remove public access to them at a later point.

We achieve this by making the actual properties private and putting them behind getter functions that ensure a console warning is issued on access. Additionally, we make the properties readonly by not providing a setter.

Examples

Screenshot 2023-09-08 at 12 15 01 PM

@jiexi jiexi requested a review from a team as a code owner September 8, 2023 18:34
@jiexi jiexi changed the title Make chainId, networkVersion, selectedAddress readonly and log warning Make chainId, networkVersion, selectedAddress readonly and log deprecation warning Sep 8, 2023
src/messages.ts Outdated Show resolved Hide resolved
@jiexi
Copy link
Contributor Author

jiexi commented Sep 8, 2023

@metamaskbot publish-preview

Copy link
Contributor

@legobeat legobeat left a comment

Choose a reason for hiding this comment

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

I don't see why not and implementation LGTM

@jiexi jiexi merged commit ede28d5 into main Sep 11, 2023
@jiexi jiexi deleted the jl/deprecate-provider-properties branch September 11, 2023 18:54
This was referenced Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants