Do not set source.isWasm attribute for old Firefox.#3412
Conversation
Codecov Report
@@ Coverage Diff @@
## next #3412 +/- ##
==========================================
+ Coverage 51.39% 51.46% +0.07%
==========================================
Files 109 109
Lines 4530 4531 +1
Branches 937 938 +1
==========================================
+ Hits 2328 2332 +4
+ Misses 2202 2199 -3
Continue to review full report at Codecov.
|
jasonLaster
left a comment
There was a problem hiding this comment.
one comment, otherwise good.
| async function fetchSources() { | ||
| const { sources } = await threadClient.getSources(); | ||
| return sources.map(createSource); | ||
| return sources.map(source => createSource(source, debuggerClient)); |
There was a problem hiding this comment.
perhaps a slightly different signature.
const supportsWasm = !!debuggerClient.mainRoot.traits.wasmBinarySource;
createSource(source, { supportsWasm }))The rationale is that when devtools moves to github it will be easier to add a supportsWasm function in debuggerClient.
There was a problem hiding this comment.
Added DebuggerClientTraits type to be used at multiple createSource callsites.
jasonLaster
left a comment
There was a problem hiding this comment.
sorry for the back and forth, I'd prefer hiding debuggerClientTraits by keeping references to it in two spots, which we can later hide when we update debuggerClient
| export function createSource(source: SourcePayload): Source { | ||
| export function createSource( | ||
| source: SourcePayload, | ||
| debuggerClientTraits: DebuggerClientTraits |
There was a problem hiding this comment.
I would prefer passing a smaller options object in: { supportsWasm: bool }. This will keep the interface smaller and more flexible
| async function fetchSources() { | ||
| const { sources } = await threadClient.getSources(); | ||
| return sources.map(createSource); | ||
| return sources.map(source => createSource(source, debuggerClientTraits)); |
There was a problem hiding this comment.
small style knit
const supportsWasm = debuggerClient.debuggerClientTraits.supportsWasm
sources.map(source => createSource(source, { supportsWasm }));|
|
||
| function newSource(_: "newSource", { source }: SourcePacket) { | ||
| actions.newSource(createSource(source)); | ||
| actions.newSource(createSource(source, debuggerClientTraits)); |
There was a problem hiding this comment.
const supportsWasm = debuggerClient.debuggerClientTraits.supportsWasm
actions.newSource(createSource(source, { supportsWasm });| if (!isEnabled("wasm")) { | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
I cannot longer reproduce #3417, removing update line number formatting switch.
Associated Issue: https://devtools-html.slack.com/archives/C3VTWQ4KZ/p1500478468288028
Old Firefox browsers cannot return binary source. So we need to disable source.isWasm for these.
Summary of Changes
Test Plan
Observe no console errors about
getSourceLineCount