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

feat: upgrade react-native-webview #7759

Merged
merged 71 commits into from
Jul 2, 2024
Merged

feat: upgrade react-native-webview #7759

merged 71 commits into from
Jul 2, 2024

Conversation

jpcloureiro
Copy link
Contributor

@jpcloureiro jpcloureiro commented Nov 10, 2023 •
edited
Loading

Description

Moving away from .patch changes on the webview

Our RNWebView fork contains the security patches.
The "Javascript Injection" patch was removed and the related patches to fix bugs introduced by that.

Jest snapshots updates to reflect the new RNWebView component

Related issues

Addresses: mobile-planning #1294

Acceptance Criteria

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've clearly explained what problem this PR is solving and how it is solved.
  • I've linked related issues
  • I've included manual testing steps
  • I've included screenshots/recordings if applicable
  • I’ve included tests if applicable
  • I’ve documented my code using JSDoc format if applicable
  • I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.
  • I’ve properly set the pull request status:
    • In case it's not yet "ready for review", I've set it to "draft".
    • In case it's "ready for review", I've changed it from "draft" to "non-draft".

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@github-actions GitHub Actions
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@jpcloureiro jpcloureiro self-assigned this Nov 10, 2023
@jpcloureiro jpcloureiro force-pushed the upgrade/react-native-webview branch 2 times, most recently from d48f81c to cc0035f Compare November 17, 2023 15:49
@jpcloureiro
Copy link
Contributor Author

Failing tests might require patching react-native-webview

react-native-webview/react-native-webview#2959 (comment)

jpcloureiro
jpcloureiro commented Nov 17, 2023
View reviewed changes
ios/Podfile.lock Outdated Show resolved Hide resolved
jpcloureiro
jpcloureiro commented Nov 17, 2023
View reviewed changes
package.json Outdated Show resolved Hide resolved
@jpcloureiro
Copy link
Contributor Author

We have a working in-app browser on both Android and iOS platforms with latest (13.6.2) react-native-webview.

Had to change the Android WebView callback that injects the web3 provider. See changes here

Warning Merging main breaks APP <-> DAPP communication.

TODO: Identify which commit from main breaks the communication.

@wachunei
Copy link
Member

Hey @jpcloureiro, as our ramp integrations rely heavily on the webview package I want to let you and all the mobile team know that we will be keeping an eye on the progress of this PR so by the time this is ready to be reviewed our QA engineer Brendan (@bkirb) confirms our integrations with providers are not broken.

Please consider adding team-ramp, needs-ramp-qa labels when you think that time has come 🙏

@jpcloureiro jpcloureiro changed the title Upgrade/react native webview RNWebView - Upgrade react-native-webview Jan 29, 2024
@jpcloureiro jpcloureiro changed the title RNWebView - Upgrade react-native-webview Upgrade react-native-webview Jan 29, 2024
@jpcloureiro
Copy link
Contributor Author

TODO: Test Blockaid feature (PPOMView)

@metamaskbot
Copy link
Collaborator

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 056cf00
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/2b51918e-e880-409e-bbe9-8001e01402cf

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@tommasini tommasini added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Jul 1, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 1, 2024

https://bitrise.io/ Bitrise

🔄🔄🔄 pr_smoke_e2e_pipeline started on Bitrise...🔄🔄🔄

Commit hash: 056cf00
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/4d3a9b70-2252-41e1-ac7c-49a6131211cf

Note

  • This comment will auto-update when build completes
  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@metamaskbot
Copy link
Collaborator

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 056cf00
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/4d3a9b70-2252-41e1-ac7c-49a6131211cf

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@jpcloureiro jpcloureiro added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Jul 1, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 1, 2024

https://bitrise.io/ Bitrise

🔄🔄🔄 pr_smoke_e2e_pipeline started on Bitrise...🔄🔄🔄

Commit hash: 056cf00
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/985c7529-68a3-4120-8816-214c0548111f

Note

  • This comment will auto-update when build completes
  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@metamaskbot
Copy link
Collaborator

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 056cf00
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/985c7529-68a3-4120-8816-214c0548111f

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@legobeat legobeat added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Jul 1, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 1, 2024

https://bitrise.io/ Bitrise

🔄🔄🔄 pr_smoke_e2e_pipeline started on Bitrise...🔄🔄🔄

Commit hash: 88ba796
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/47839da8-01c5-4c78-81cc-d272db1bb786

Note

  • This comment will auto-update when build completes
  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Jul 1, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
95.8% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@metamaskbot
Copy link
Collaborator

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 88ba796
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/47839da8-01c5-4c78-81cc-d272db1bb786

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

2 similar comments
@metamaskbot
Copy link
Collaborator

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 88ba796
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/47839da8-01c5-4c78-81cc-d272db1bb786

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@metamaskbot
Copy link
Collaborator

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 88ba796
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/47839da8-01c5-4c78-81cc-d272db1bb786

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@jpcloureiro jpcloureiro added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Jul 2, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 2, 2024

https://bitrise.io/ Bitrise

🔄🔄🔄 pr_smoke_e2e_pipeline started on Bitrise...🔄🔄🔄

Commit hash: 88ba796
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/baad678e-67dc-441c-949d-b25057555574

Note

  • This comment will auto-update when build completes
  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@metamaskbot
Copy link
Collaborator

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 88ba796
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/baad678e-67dc-441c-949d-b25057555574

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

2 similar comments
@metamaskbot
Copy link
Collaborator

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 88ba796
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/baad678e-67dc-441c-949d-b25057555574

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@metamaskbot
Copy link
Collaborator

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 88ba796
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/baad678e-67dc-441c-949d-b25057555574

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

FrederikBolding
FrederikBolding approved these changes Jul 2, 2024
View reviewed changes
Copy link
Member

@FrederikBolding FrederikBolding left a comment

Choose a reason for hiding this comment

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

Snaps changes LGTM and confirmed working 👍

@jpcloureiro jpcloureiro merged commit 67eb692 into main Jul 2, 2024
43 of 44 checks passed
@jpcloureiro jpcloureiro deleted the upgrade/react-native-webview branch July 2, 2024 11:55
@github-actions github-actions bot locked and limited conversation to collaborators Jul 2, 2024
@github-actions github-actions bot removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Jul 2, 2024
@metamaskbot metamaskbot added the release-7.27.1 Issue or pull request that will be included in release 7.27.1 label Jul 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@tommasini tommasini tommasini approved these changes

@jpuri jpuri jpuri approved these changes

@FrederikBolding FrederikBolding FrederikBolding approved these changes

@brianacnguyen brianacnguyen brianacnguyen left review comments

@wachunei wachunei Awaiting requested review from wachunei

@legobeat legobeat Awaiting requested review from legobeat

@Gudahtt Gudahtt Awaiting requested review from Gudahtt

@naugtur naugtur Awaiting requested review from naugtur

Assignees

@jpcloureiro jpcloureiro

Labels
needs-ramp-qa Tickets that need feature QA on the ramps flows release-7.27.1 Issue or pull request that will be included in release 7.27.1 RNWebView Run Smoke E2E Triggers smoke e2e on Bitrise team-mobile-platform team-ramp issues related to Ramp features
Projects
PR review queue
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.