fix: Patch react-devtools-core to support jsdom-jscore-rn#47616
fix: Patch react-devtools-core to support jsdom-jscore-rn#47616
Conversation
The native editor defining `window.document` caused `react-devtools-core` to believe the environment was a browser, not React Native. This resulted in the following error when inspecting components using the React Dev Tools: Android: ``` Cannot set property 'zIndex' of undefined ``` iOS: ``` TypeError: undefined is not an object (evaluating 'this.container.style.zIndex = '10000000'') ```
|
Size Change: +367 B (0%) Total Size: 1.31 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 43ebebc. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4055978043
|
derekblank
left a comment
There was a problem hiding this comment.
No longer receiving zIndex warnings after testing. LGTM! Thanks for tackling this one. 🚀
|
@dcalhoun Would a corresponding Gutenberg Mobile PR be expected to pair with this PR? I see that the Automation label was not included. Just curious if that was intentional based on the patched nature of these changes, or if there is another reason (or, is simply not needed). |
I chose to forgo a Gutenberg Mobile PR as (1) I was less concerned these developer tool centric changes would introduce CI test suite breakages and (2) I can include this change via wordpress-mobile/gutenberg-mobile#5439 if I merge this PR before resolving the former's merge conflict by including the latest It could be argued I should've still opened Gutenberg Mobile PR for this PR to leave an explicit "paper trail" of the inclusion of this change. However, many Gutenberg commits are brought into Gutenberg Mobile via Dependabot PRs or arbitrary updates to the Gutenberg submodule reference. So, I figure it is not an issue. |
This patch was originally created to prevent an warning caused by the project's use of `jsdom-jscore-rn`. #47616
* build: Capture result of @rnx-kit/align-deps Result of running the following: ``` npx @rnx-kit/align-deps@latest --write --requirements react-native@0.73 ``` * build: Revert incompatible @rnx-kit/align-deps changes Revert changes that are incompatible with the Gutenberg project because they are a undesired downgrade or erroneously modify usage of the `^` version annotation. * build: Replace Metro packages with @react-native namespace equivalents These packages were renamed, the old names are deprecated. * build: Capture package lock file changes The result of `npm install` after update React Native-related dependencies for 0.73. * build: Recreate react-native patch file The result of running `npx patch-package react-native`. This patch exists as a workaround for the following RN core issue: facebook/react-native#36465 * build: Correct lock file changes The previous lock file was incorrect, as it was the result of running `npm install` after merely bumping package version numbers. In order to capture the correct lock file results, one must follow the Gutenberg project's guidelines for updating dependencies, which involves removing and re-adding each dependency. This now captures the result of `npm install` after following that process. https://github.com/WordPress/gutenberg/tree/c7cf2f74f38ae4c1f325e3238e61270a801f183c/packages#updating-existing-dependencies * build: Replace overlooked deprecated Metro packages names These packages were renamed, the old names are deprecated. * build: Recreate react-devtools-core patch This patch was originally created to prevent an warning caused by the project's use of `jsdom-jscore-rn`. #47616 * build: Expand react-devtools-core patch to cover all window references This should improve React Devtool support for the React Native project. Likely enabling features like highlighting a UI element when hovering its component in the devtools. * build: Migrate to @react-native/metro-config Metro changed its approach for extending the default configuration. * fix: Disable the optional Demo editor setup configuration This logic threw the following error after upgrading to React Native 0.73. We should follow up to fix the error or remove the logic. https://github.com/facebook/metro/blob/877933ddc03672a00214d26afe620b79479d6489/packages/metro-file-map/src/lib/TreeFS.js#L417 ``` Unexpectedly escaped traversal ``` * Update Sass transformer to include workaround when being loaded from Gutenberg Mobile, to appened the right directory path * Fix Reanimated test mock * Add experimental debugger flag * Update Reanimated * build: Update Gemfile for React Native 0.73 * build: Bump minimum iOS version to 13.4 This was done to address the following error when installing Pods: ``` [!] CocoaPods could not find compatible versions for pod "React-FabricImage": In Podfile: React-FabricImage (from `../../../node_modules/react-native/ReactCommon`) Specs satisfying the `React-FabricImage (from `../../../node_modules/react-native/ReactCommon`)` dependency were found, but they required a higher minimum deployment target. ``` * build: Update Podfile for React Native 0.73 * build: Upgrade to react-native-reanimated 3.6.2 Required to address the following error. https://stackoverflow.com/a/77257722/378228 ``` No template named 'optional' in namespace 'std'; did you mean 'folly::Optional'? ``` * build: Update Info.plist NSAppTransportSecurity Align with the latest React Native template to avoid future conflicts. * build: Align dependencies with react-native-libraries-publisher The react-native-libraries-publisher provides published copies of these dependencies that are consumed by the host WordPress app. The versions here must align with the versions captured there. #58475 (comment) * build: Capture latest Pod changes * Remove @react-native/gradle-plugin since it's included in the React Native package with support for AGP 8.2.1 * Migrate java files to kotlin * Update build and gradle files * Remove Flipper java file * Update debug AndroidManifest * Update react-native-config.js to include all packages that we are manually linking. This is needed to be able to use React Native's applyNativeModulesAppBuildGradle which includes needed configuration for other integration things. This auto links modules so we need to skip the ones we manually link. * Update package-lock.json * Updates react-native.config to remove disabling iOS auto-linking * Updates the comment explaning why auto-linking is disabled for Android in the react-native.config.js file * build: Configure GitHub Action ruby version Previously, the CI would load the default ruby version, which caused unexpected outcomes. * test: Update Reanimated warning during tests This existing warning was reworded. * test: Fix failing Gallery edit test Restore mock to its original implementation rather than resetting it to an empty function. It is not entirely clear why this test began failing after upgrading to React Native 0.73. * build: Upgrade to @testing-library/react-native@12.4.3 * test: Avoid duplicate call to mock internal `TextInputState` module We already invoke `jest.mock()` for this module within the Jest environment setup file. We do so as _not_ mocking this module causes widespread failures in tests due to its relationship to native modules. So, in a way, it makes sense to globally mock it. For whatever reason, attempting to invoke `jest.mock()` twice for this module stopped working after upgrading to React Native 0.73. The mock function passed to the second invocation was ignored. This change now updates the existing mock instead. * test: Update LinkUI snapshot The specific cause for this update was not determined, but seemingly the ref is no longer passed to the navigation `Stack.Screen` component. There is no reason to believe this is an issue at this time. * fix: Allow animation start during mount This was done to fix failing automated tests. The guard was originally added to prevent invoking the animation end callback, so this should be safe to change. * test: Leverage fake timers for Draggable animation tests Without fake timers, the callbacks are never invoked as the animation never completes. This outcome started happening after upgrading to `react-native-reanimated@3.x`. * test: Await missing block render from running timers Addresses an act warning during the test run that started after upgrading to React Native 0.73. * test: Update timer mocks for Reanimated 3.x The navigation container test failed due to `act` warnings and broken mocked timers. Wrapping timer runs with `act` address the former. The latter is addressed by no longer relying upon "legacy" mock timers, simplifying the approach for advancing the timers for animations, and removing the global `requestAnimationFrame` override. All of these same actions were applied to the Reanimated library in a refactor: software-mansion/react-native-reanimated@99573c3 * test: Refactor advanceAnimationByFrames utility Align the implementation with the Reanimated version. It is worth noting the Reanimated utility is marked as deprecated. * test: Fix failing fake timer detection The previous approach using `jest.isMockFunction` failed after upgrading to React Native 0.73. While it is not clear why it began failing, the new approach is sourced from the `@testing-library/react-native` source. https://github.com/callstack/react-native-testing-library/blob/a670b2d4c1fb4df5326a63cb2852f4d6e37756da/src/helpers/timers.ts#L24-L58 * Revert upgrading to AGP 8.1.0 until the main Android host app upgrades to 8.1.1. We can't have different AGP versions and if we do this change the main host app will ned to update as well as their dependencies. For now we'll fix all dependencies AGP versions in the React Native demo app. * ci: Restrict `ruby/setup-ruby` CI action to specific commit hash Mitigate Ci instability or insecurity. * Update Changelog --------- Co-authored-by: Gerardo <gerardo.pacheco@automattic.com>
What?
Patch
react-devtools-coreto supportjsdom-jscore-rn, which is used by thenative mobile editor.
Why?
The native editor defines
window.documentwhich causedreact-devtools-coreto believe the environment was a browser, not React Native. This resulted in the
following error when inspecting components using the React Dev Tools:
Android:
iOS:
How?
Add a custom property to
window.documentto informreact-devtools-corethatthe environment is using
jsdomand it should not presume it is a browserenvironment with a fully capable DOM.
Testing Instructions
npm run native start:resetnpm run native androidreact-devtoolsvia CLI.zIndexwarnings due not display on the device orthe server log.
Testing Instructions for Keyboard
n/a
Screenshots or screencast
n/a