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

Jump to bottom

Remove web3 injection #9156

Merged
merged 5 commits into from
Dec 7, 2020
Merged

Remove web3 injection #9156

merged 5 commits into from
Dec 7, 2020

Conversation

rekmarks
Copy link
Member

@rekmarks rekmarks commented Aug 7, 2020 •
edited
Loading

  • Removes window.web3 injection and all related code
  • Re-implements logWeb3Usage as logWeb3ShimUsage, which will be called from the web3 shim in the inpage-provider
    • For this to work, the inpage-provider must be updated. Until then (very soon), it will merely be unused.

Closes #7163

@rekmarks rekmarks added the DO-NOT-MERGE Pull requests that should not be merged label Aug 7, 2020
@rekmarks rekmarks force-pushed the web3-removal branch 3 times, most recently from 192f788 to 7851da0 Compare August 10, 2020 15:26
@rekmarks rekmarks mentioned this pull request Aug 10, 2020
Merged
@rekmarks rekmarks force-pushed the web3-removal branch 2 times, most recently from 623e154 to 1084ad2 Compare August 10, 2020 16:50
@metamaskbot
Copy link
Collaborator

Builds ready [1084ad2]
Page Load Metrics (560 ± 49 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint29403432
domContentLoaded24962055910149
load25062156010149
domInteractive24861955810149

@metamaskbot
Copy link
Collaborator

Builds ready [a45814e]
Page Load Metrics (580 ± 63 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint319943188
domContentLoaded28478657913163
load28678858013063
domInteractive28478657813163

@rekmarks rekmarks mentioned this pull request Aug 17, 2020
Closed
@metamaskbot
Copy link
Collaborator

Builds ready [0052263]
Page Load Metrics (610 ± 54 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint319146209
domContentLoaded35982860711354
load36382861011254
domInteractive35882760711354

@metamaskbot
Copy link
Collaborator

Builds ready [0de5f29]
Page Load Metrics (415 ± 57 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint288036115
domContentLoaded30961741311857
load31061841511857
domInteractive30861741311857

@Gudahtt Gudahtt mentioned this pull request Aug 21, 2020
Closed
@MetaMask MetaMask locked and limited conversation to collaborators Sep 7, 2020
@MetaMask MetaMask deleted a comment from metamaskbot Sep 7, 2020
@metamaskbot
Copy link
Collaborator

Builds ready [cf3730d]
Page Load Metrics (488 ± 74 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint30109542713
domContentLoaded31481148615474
load31581248815474
domInteractive31381148615474

@danjm danjm added this to the v8.1.next? milestone Nov 2, 2020
@rekmarks rekmarks force-pushed the web3-removal branch 2 times, most recently from 2dc1a27 to 447ea1b Compare November 24, 2020 05:39
@metamaskbot
Copy link
Collaborator

Builds ready [447ea1b]
Page Load Metrics (452 ± 36 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint2985462110
domContentLoaded3075344507536
load3125354527436
domInteractive3075344507536

kumavis
kumavis previously approved these changes Nov 26, 2020
View reviewed changes
@kumavis
Copy link
Member

kumavis commented Nov 26, 2020

seems clean

@metamaskbot
Copy link
Collaborator

Builds ready [fdfbd5d]
Page Load Metrics (492 ± 39 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint30110502311
domContentLoaded3225874908239
load3245884928239
domInteractive3225874908239

@rekmarks rekmarks force-pushed the web3-removal branch 2 times, most recently from 5e07a1d to 15d716d Compare December 2, 2020 20:50
@rekmarks rekmarks requested a review from Gudahtt December 2, 2020 20:59
@rekmarks rekmarks marked this pull request as ready for review December 2, 2020 21:00
@rekmarks rekmarks requested a review from a team as a code owner December 2, 2020 21:00
@rekmarks rekmarks removed the DO-NOT-MERGE Pull requests that should not be merged label Dec 2, 2020
@metamaskbot
Copy link
Collaborator

Builds ready [fe13a73]
Page Load Metrics (529 ± 44 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint3197642210
domContentLoaded3236305269144
load3256315299144
domInteractive3236295269144

@metamaskbot
Copy link
Collaborator

Builds ready [e1aec0e]
Page Load Metrics (378 ± 45 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint297249167
domContentLoaded2776493769345
load2786513789345
domInteractive2776493759344

@rekmarks rekmarks requested a review from kumavis December 5, 2020 04:39
@brad-decker brad-decker assigned brad-decker and unassigned Gudahtt Dec 7, 2020
brad-decker
brad-decker approved these changes Dec 7, 2020
View reviewed changes
Copy link
Contributor

@brad-decker brad-decker left a comment

Choose a reason for hiding this comment

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

LGTM. I tested on a number of production dapps to ensure window.web3 was undefined and was able to still connect, etc.

@rekmarks rekmarks merged commit 6795298 into develop Dec 7, 2020
@rekmarks rekmarks deleted the web3-removal branch December 7, 2020 20:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@brad-decker brad-decker brad-decker approved these changes

@Gudahtt Gudahtt Awaiting requested review from Gudahtt

@kumavis kumavis Awaiting requested review from kumavis

Assignees

@brad-decker brad-decker

Labels
None yet
Projects
None yet
Milestone
v8.1.next?
Development

Successfully merging this pull request may close these issues.

Stop injecting web3.js
6 participants
@rekmarks @metamaskbot @kumavis @brad-decker @Gudahtt @danjm