refactor(inspector): use Object.defineProperty(s) for console and network hooks #101
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…work hooks - Add sub-agent-console-network for web/RN console and network hook guidance - runtime.ts: install/restore console hooks via Object.defineProperties and getOwnPropertyDescriptor - network.ts: install/restore XHR and fetch via Object.defineProperty; add DOM lib reference; fix open and this types
📊 Test Coverage Report - React Native InspectorSummary
File CoverageClick to expand
Generated by test coverage script |
- Guard onopen with disconnectRequested so open does not set sendFn after disconnect - In onclose: clear ws and sendFn first, then return early if disconnectRequested; otherwise schedule retry
📊 Test Results✅ Test Status: PASSED📸 ScreenshotsScreenshots are available as artifacts. Download them here. Artifact name: Screenshot files
|
…all and restore - Console: descriptor backup/restore via defineProperties; idempotent second enable - Network: XMLHttpRequest and fetch replace/restore via defineProperty; idempotent second enable
There was a problem hiding this comment.
Pull request overview
This PR refactors console and network hooks in the React Native Inspector to use Object.defineProperty and Object.defineProperties APIs instead of direct assignment. It also adds sub-agent documentation to guide the same pattern for both web and React Native clients.
Changes:
- Added sub-agent documentation (
.cursor/agents/sub-agent-console-network.mdc) describing the Object API pattern for console and network hooks - Updated console hooks in
runtime.tsto useObject.definePropertiesfor installation/restoration and save original property descriptors - Updated network hooks in
network.tsto useObject.definePropertyfor XMLHttpRequest and fetch, with descriptor backup and improved typing (XHR.open parameters, fetchthiscontext)
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
.cursor/agents/sub-agent-console-network.mdc |
Sub-agent documentation defining the Object API pattern for console and network hooks across all clients |
packages/react-native-inspector/src/cdp/domain/runtime.ts |
Refactored console hooks to use Object.defineProperties with descriptor backup/restore |
packages/react-native-inspector/src/cdp/domain/network.ts |
Refactored XMLHttpRequest and fetch hooks to use Object.defineProperty with descriptor backup/restore, added DOM type reference, fixed typing for XHR.open and fetch wrapper |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Integration Tests1 tests 1 ✅ 18s ⏱️ Results for commit 6dd10fb. ♻️ This comment has been updated with latest results. |
📱 Maestro Android Test Results✅ PASSED Maestro Android tests have completed. Check the workflow run for details. |
📱 Maestro iOS Test Results✅ PASSED Maestro iOS tests have completed. Check the workflow run for details. |
- runtime.ts: reset originalConsole/originalDescriptors on defineProperties failure; clear backups only after successful uninstall restoration - network.ts: clear originalXHR/originalXHRDescriptor on defineProperty failure in hookXHR; clear backup refs only after both XHR and fetch restorations succeed in uninstallHooks; document __ChromeRemoteDevToolsHooked - sub-agent-console-network.mdc: clarify Object API pattern is RN-only for now
- Treeoutline: skip hasSelection check for expandable nodes so caret does not block toggle; allow toggle on any row click when expandable (isInTriangle/toggleOnClick may fail in WebKit) - Apply same logic in bundled Treeoutline.js (Inspector loads bundled)
- DevToolsPlayground testConsole: log object { count: 1, status: 'ok' }
for testing console row expand/collapse (e.g. Safari/WebKit)
- en/ko examples: document object log in Test Console and example code
ChromeRemoteDevToolsInspector native pod removed; RN inspector is JavaScript-only.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Align React Native inspector CDP with web client structure
Purpose
Align the React Native inspector’s CDP message handling (console, network) with the web client’s folder structure and code shape so both codebases use the same patterns and naming.
Description
Folder structure (aligned with web
packages/client/src/cdp/)cdp/domain/protocol.ts: Event constants (e.g.Runtime.consoleAPICalled,Network.requestWillBeSent).cdp/domain/base.ts: Shared send path (sendCDPEvent,setCDPEventSender,setCDPConnectionReady), analogous to webBaseDomain.send.cdp/domain/runtime.ts: Console hook logic (enable/disable/isEnabled).cdp/domain/network.ts: Network hook logic (XHR/fetch).cdp/common/value-to-remote-object.ts: JS value → CDP RemoteObject (same role as webcommon/remoteObject).Single send API
setCDPEventSenderandsetCDPConnectionReady.setConsoleCDPSender/setNetworkCDPSenderremoved.cdp-message.tsre-exports fromcdp/domain/base;connect()callssetCDPEventSender(sender)andsetCDPConnectionReady()once.Backward compatibility
console/andnetwork/re-export fromcdp/domain/runtimeandcdp/domain/network.valueToRemoteObjectis exported fromcdp/common/value-to-remote-objectand re-exported fromconsole/index.Tests
setCDPEventSender/setCDPConnectionReadyand imports fromcdp/domain/runtimeandcdp/domain/network.cdp/common/value-to-remote-object.cdp-domain-base.test.ts(send path: null sender, not ready, missing server info, successful send, serialize failure) andcdp-domain-protocol.test.ts(Event constant values).Test coverage includes: console-hook, network-hook, value-to-remote-object, cdp/domain/base, cdp/domain/protocol.
How to test
bun run format,bun run lint, thenbun test packages/react-native-inspector/src/__tests__Copilot review (applied)
Object.definePropertiesfailure, resetoriginalConsoleandoriginalDescriptorsso the next call does not see inconsistent state.Object.definePropertiesrestoration; on failure returnfalsewithout clearing so retry can restore.Object.definePropertyfailure, clearoriginalXHRandoriginalXHRDescriptorso a retry does not use partially-hooked values.originalXHR/originalFetch(and descriptors) to null/undefined after both XHR and fetch restorations succeed; on exception keep backups.Safari/WebKit console object expand (fix)
hasSelection()was true (caret) andisInTriangle/toggleOnClickcould be false. For expandable nodes, selection is no longer checked (so caret does not block toggle), and toggle is allowed on any row click. Applied in bundledTreeoutline.jsso Inspector (which loads from bundled) gets the fix.Web playground console object test (docs)
{ count: 1, status: 'ok' }for testing row expand/collapse (e.g. Safari).Additional info
bun run formatandbun run lintbefore committing.commit-summary-refactor-inspector-rn-cdp.md.