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

Define global web3 as non-enumerable #8634

Merged
merged 1 commit into from
May 21, 2020

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented May 20, 2020

We inject web3 globally on most websites. This has been breaking websites that attempted to serialize the window object, because any attempt to access certain web3 properties (such as web3.eth.mining) would throw an error. This is because web3 defined a getter for these properties that would call .send([method]), which doesn't work for most methods.

An example of a site that this breaks is Storybook, when the @storybook/addon-actions addon is being used. When using storybook with this addon and with the MetaMask extension installed, actions would not be properly dispatched because an error would be thrown in the attempt to serialize the event (which includes a reference to the window).

The web3 global we inject is now defined as non-enumerable, so it will be skipped automatically in any attempt to serialize the window object.

We inject `web3` globally on most websites. This has been breaking
websites that attempted to serialize the `window` object, because any
attempt to access certain `web3` properties (such as `web3.eth.mining`)
would throw an error. This is because `web3` defined a getter for these
properties that would call `.send([method])`, which doesn't work for
most methods.

An example of a site that this breaks is `Storybook`, when the
`@storybook/addon-actions` addon is being used. When using storybook
with this addon and with the MetaMask extension installed, actions
would not be properly dispatched because an error would be thrown in
the attempt to serialize the event (which includes a reference to the
`window`).

The `web3` global we inject is now defined as non-enumerable, so it
will be skipped automatically in any attempt to serialize the `window`
object.
@Gudahtt Gudahtt force-pushed the define-web3-global-as-non-enumerable branch from 224791f to c447454 Compare May 21, 2020 02:33
@Gudahtt Gudahtt marked this pull request as ready for review May 21, 2020 02:50
@Gudahtt Gudahtt requested a review from a team as a code owner May 21, 2020 02:50
@metamaskbot
Copy link
Collaborator

Builds ready [c447454]
Page Load Metrics (591 ± 48 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint319341157
domContentLoaded34972859010148
load35172959110148
domInteractive34972858910149

@Gudahtt
Copy link
Member Author

Gudahtt commented May 21, 2020

We'll be stopping the web3 injection soon hopefully (see #7163), but this should suffice in the meantime.

@danfinlay danfinlay merged commit 9e2e353 into develop May 21, 2020
@danfinlay danfinlay deleted the define-web3-global-as-non-enumerable branch May 21, 2020 03:18
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.

4 participants