fix(tool-server): fall back to raw locations when inspect-element symbolication fails#284
Draft
latekvo wants to merge 1 commit into
Draft
fix(tool-server): fall back to raw locations when inspect-element symbolication fails#284latekvo wants to merge 1 commit into
latekvo wants to merge 1 commit into
Conversation
…bolication fails debugger-inspect-element with the default resolveSourceMaps:true returned source:null and code:null for every item whenever Metro /symbolicate either failed or mapped a frame into node_modules. The resolveSourceMaps branch only assigned source/code when symbolicate succeeded, so a null result discarded the raw frame location the tool already had. Two minimal fixes: - debugger-inspect-element: when symbolicate returns null, fall back to the raw bundled frame (file, line, column) — mirroring the resolveSourceMaps:false shape — instead of leaving source/code null. - source-resolver: the symbolicate guard rejected any path containing "node_modules", which threw away genuinely-mapped sources for route components that legitimately live in node_modules (expo-router / react-navigation). Replace it with a check that only rejects unmapped bundle URLs Metro echoes back (http(s) URLs); real file paths, including node_modules ones, are kept. Adds resolver-level coverage for the node_modules-kept and echoed-URL-rejected cases, plus a tool-level test asserting the raw-frame fallback when symbolication fails.
3117801 to
1d447e5
Compare
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
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.
Problem
debugger-inspect-element, called with the defaultresolveSourceMaps: true, returnedsource: nullandcode: nullfor every item whenever Metro symbolication did not yield a usable original location. This made the tool effectively useless in its default mode — the caller got a list of component names with no file:line and no code fragment to edit.Root cause
Two independent issues compounded:
No raw fallback in the tool. In
debugger-inspect-element.ts, theresolveSourceMapsbranch only assignedsource/codewhensymbolicate()returned a result. When it returnednull(symbolication failed, or was filtered out — see below), the item was left withsource: null/code: null, discarding the raw bundled frame the tool already had initem.frame.Over-broad
node_modulesrejection in the resolver. Insource-resolver.ts,symbolicateFramedidif (frame.file.includes("node_modules")) return null;. That was meant to drop frames that symbolication couldn't map (where Metro echoes the bundle URL back). But it also threw away genuinely-mapped sources whose real path legitimately lives innode_modules— e.g.expo-router/react-navigationroute components — turning a perfectly good resolved location intonull, which then hit issue (1) and nulled the whole item.Fix
Two minimal, localized changes:
debugger-inspect-element.ts— add anelseto theresolveSourceMapsbranch that falls back to the raw bundled frame{ file: item.frame.file, line: item.frame.line, column: item.frame.col }, mirroring the exact shape produced by theresolveSourceMaps: falsebranch. When symbolication can't map a frame, the caller now still gets the bundled location instead ofnull.source-resolver.ts(line ~93) — replace theincludes("node_modules")guard withif (/^https?:\/\//.test(frame.file)) return null;. A successful symbolication yields a real file path (kept, including legitimatenode_modulesroute components); a failed one yields the originalhttp(s)bundle URL echoed back (rejected). This keeps the diff to a single line + comment at line ~93 to stay easy to reconcile.Testing
cd packages/tool-server && vitest run test/metro/source-resolver.test.ts test/debugger/inspect-element-filter.test.tsAdded fail-before / pass-after coverage at two layers:
Resolver level (
test/metro/source-resolver.test.ts): mocksfetchfor/symbolicateand asserts thatnode_modules/.../*.jspath is kept (returned that mapped, project-relative location),http(s)bundle URL is rejected (null),null.Tool level (
test/debugger/inspect-element-filter.test.ts): drivesdebuggerInspectElementTool.execute()with a fakeJsRuntimeDebuggerApi(no live CDP) and asserts that whensymbolicate()returnsnull, the item retains the raw{ file, line, column }(and reads no source fragment); when it returns a mapped location, that location and its code fragment are used.Verified the fail-before / pass-after cycle by checking out
origin/mainfor the two source files only (keeping the new tests): the 3 new assertions fail onorigin/main(sourceisnullfor the raw-fallback case; the node_modules-mapped source isnull; the echoed bundle URL is wrongly accepted), then pass once the fixes are restored. All 34 tests pass with the fix; the 31 pre-existing tests are unaffected.tsc --noEmit -p tsconfig.test.jsonreports no errors in any changed file (the only errors are pre-existing module-resolution artifacts from the symlinkednode_modules:pngjs,@argent/native-devtools-android,@argent/update-core).Known limitation / follow-up
User-defined and route component frames that lack
_debugStackunder React 19 (where_debugSourcewas removed) are stillnulleven in raw mode, because there is no bundled frame to fall back to in the first place. That is a separate, riskier change (it requires a different source for the frame entirely) and is intentionally out of scope here.Note for reconcilers
There are other open branches touching
source-resolver.ts(fix/source-resolver-traversal,fix/source-map-ssrf). This change is deliberately confined to the single guard at line ~93 (plus an explanatory comment) to keep it trivially mergeable alongside them.