Skip to content

Commit

Permalink
Fix misaligned lines in debugger (#634)
Browse files Browse the repository at this point in the history
This PR fixes a problem with debugger on expo projects, caused by
prelude lines added to the entry bundle file added by expo. The solution
is t0 add the required offset to the prelude causing the problem and
scanning for it when receiving source map in the debuger.

It is possible that the changes will no longer be needed in future
versions of react native as the expo team tries to correct source map
generation to include extra lines, for more information read those PRs:
- [expo side ](expo/expo#29463)
- [metro side](facebook/metro#1284)

This is why we add version check before adding lineOffset to initial
source map.

### How Has This Been Tested: 
- open expo project and set a breakpoint, before the changes it would
trigger 2 lines above the place it's been set.
- save any change in the file containing breakpoint and make sure it
still works.
- check if links to files generated next to console logs are pointing in
to the correct place, again both before and after making changes to the
file.
- check if error position indicator (when uncaught exception is raised)
points to the correct position.


### Additional changes: 
This PR also fixes a minor mistake with `getReactNativeVersion` utility
that does not need to be async and as it is used, we make it synchronous
as part of this PR.

---------

Co-authored-by: Krzysztof Magiera <[email protected]>
  • Loading branch information
filip131311 and kmagiera authored Oct 22, 2024
1 parent 6544ae4 commit 1023dc7
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 19 deletions.
20 changes: 12 additions & 8 deletions packages/vscode-extension/lib/metro_helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,19 @@ function adaptMetroConfig(config) {
}
} else if (module.path === "__env__") {
// this handles @expo/env plugin, which is used to inject environment variables
// the code below instantiates a global variable __EXPO_ENV_PRELUDE_LINES__ that stores
// the number of lines in the prelude. This is used to calculate the line number offset
// when reporting line numbers from the JS runtime. The reason why this is needed, is that
// the code below exposes the number of lines in the prelude.
// This is used to calculate the line number offset
// when reporting line numbers from the JS runtime, breakpoints
// and uncaught exceptions. The reason why this is needed, is that
// metro doesn't include __env__ prelude in the source map resulting in the source map
// transformation getting shifted by the number of lines in the prelude.
const expoEnvCode = module.output[0].data.code;
if (!expoEnvCode.includes("__EXPO_ENV_PRELUDE_LINES__")) {
module.output[0].data.code = `${expoEnvCode};var __EXPO_ENV_PRELUDE_LINES__=${module.output[0].data.lineCount};`;
}
// transformation getting shifted by the number of lines in the expo generated prelude.
process.stdout.write(
JSON.stringify({
type: "RNIDE_expo_env_prelude_lines",
lineCount: module.output[0].data.lineCount,
})
);
process.stdout.write("\n");
}
return origProcessModuleFilter(module);
};
Expand Down
3 changes: 1 addition & 2 deletions packages/vscode-extension/lib/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ function wrapConsole(consoleFunc) {
const stack = parseErrorStack(new Error().stack);
const expoLogIndex = stack.findIndex((frame) => frame.methodName === "__expoConsoleLog");
const location = expoLogIndex > 0 ? stack[expoLogIndex + 1] : stack[1];
const lineOffset = global.__EXPO_ENV_PRELUDE_LINES__ || 0;
args.push(location.file, location.lineNumber - lineOffset, location.column);
args.push(location.file, location.lineNumber, location.column);
return consoleFunc.apply(console, args);
};
}
Expand Down
2 changes: 1 addition & 1 deletion packages/vscode-extension/src/builders/buildAndroid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ export async function buildAndroid(
),
];
// configureReactNativeOverrides init script is only necessary for RN versions older then 0.74.0 see comments in configureReactNativeOverrides.gradle for more details
if (semver.lt(await getReactNativeVersion(), "0.74.0")) {
if (semver.lt(getReactNativeVersion(), "0.74.0")) {
gradleArgs.push(
"--init-script", // configureReactNativeOverrides init script is used to patch React Android project, see comments in configureReactNativeOverrides.gradle for more details
path.join(
Expand Down
40 changes: 33 additions & 7 deletions packages/vscode-extension/src/debugging/DebugAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import {
Source,
StackFrame,
} from "@vscode/debugadapter";
import { getReactNativeVersion } from "../utilities/reactNative";

Check warning on line 17 in packages/vscode-extension/src/debugging/DebugAdapter.ts

View workflow job for this annotation

GitHub Actions / check

`../utilities/reactNative` import should occur after import of `source-map`
import semver from "semver";
import { DebugProtocol } from "@vscode/debugprotocol";
import WebSocket from "ws";
import { NullablePosition, SourceMapConsumer } from "source-map";
Expand Down Expand Up @@ -83,8 +85,8 @@ export class DebugAdapter extends DebugSession {
private absoluteProjectPath: string;
private projectPathAlias?: string;
private threads: Array<Thread> = [];
private sourceMaps: Array<[string, string, SourceMapConsumer]> = [];

private sourceMaps: Array<[string, string, SourceMapConsumer, number]> = [];
private expoPreludeLineCount: number;
private linesStartAt1 = true;
private columnsStartAt1 = true;

Expand All @@ -96,6 +98,7 @@ export class DebugAdapter extends DebugSession {
this.absoluteProjectPath = configuration.absoluteProjectPath;
this.projectPathAlias = configuration.projectPathAlias;
this.connection = new WebSocket(configuration.websocketAddress);
this.expoPreludeLineCount = configuration.expoPreludeLineCount;

this.connection.on("open", () => {
// the below catch handler is used to ignore errors coming from non critical CDP messages we
Expand Down Expand Up @@ -150,7 +153,30 @@ export class DebugAdapter extends DebugSession {
const decodedData = Buffer.from(base64Data, "base64").toString("utf-8");
const sourceMap = JSON.parse(decodedData);
const consumer = await new SourceMapConsumer(sourceMap);
this.sourceMaps.push([message.params.url, message.params.scriptId, consumer]);

// We detect when a source map for the entire bundle is loaded by checking if __prelude__ module is present in the sources.
const isMainBundle = sourceMap.sources.includes("__prelude__");

// Expo env plugin has a bug that causes the bundle to include so-called expo prelude module named __env__
// which is not present in the source map. As a result, the line numbers are shifted by the amount of lines
// the __env__ module adds. If we detect that main bundle is loaded, but __env__ is not there, we use the provided
// expoPreludeLineCount which reflects the number of lines in __env__ module to offset the line numbers in the source map.
const bundleContainsExpoPrelude = sourceMap.sources.includes("__env__");
let lineOffset = 0;
if (isMainBundle && !bundleContainsExpoPrelude && this.expoPreludeLineCount > 0) {
Logger.debug(
"Expo prelude lines were detected and an offset was set to:",
this.expoPreludeLineCount
);
lineOffset = this.expoPreludeLineCount;
}

this.sourceMaps.push([
message.params.url,
message.params.scriptId,
consumer,
lineOffset,
]);
this.updateBreakpointsInSource(message.params.url, consumer);
}

Expand Down Expand Up @@ -264,7 +290,7 @@ export class DebugAdapter extends DebugSession {
let sourceLine1Based = lineNumber1Based;
let sourceColumn0Based = columnNumber0Based;

this.sourceMaps.forEach(([url, id, consumer]) => {
this.sourceMaps.forEach(([url, id, consumer, lineOffset]) => {
// when we identify script by its URL we need to deal with a situation when the URL is sent with a different
// hostname and port than the one we have registered in the source maps. The reason for that is that the request
// that populates the source map (scriptParsed) is sent by metro, while the requests from breakpoints or logs
Expand All @@ -273,7 +299,7 @@ export class DebugAdapter extends DebugSession {
if (id === scriptIdOrURL || compareIgnoringHost(url, scriptIdOrURL)) {
scriptURL = url;
const pos = consumer.originalPositionFor({
line: lineNumber1Based,
line: lineNumber1Based - lineOffset,
column: columnNumber0Based,
});
if (pos.source != null) {

Check warning on line 305 in packages/vscode-extension/src/debugging/DebugAdapter.ts

View workflow job for this annotation

GitHub Actions / check

Expected '!==' and instead saw '!='
Expand Down Expand Up @@ -440,7 +466,7 @@ export class DebugAdapter extends DebugSession {
}
let position: NullablePosition = { line: null, column: null, lastColumn: null };
let originalSourceURL: string = "";
this.sourceMaps.forEach(([sourceURL, scriptId, consumer]) => {
this.sourceMaps.forEach(([sourceURL, scriptId, consumer, lineOffset]) => {
const sources = [];
consumer.eachMapping((mapping) => {
sources.push(mapping.source);
Expand All @@ -453,7 +479,7 @@ export class DebugAdapter extends DebugSession {
});
if (pos.line != null) {

Check warning on line 480 in packages/vscode-extension/src/debugging/DebugAdapter.ts

View workflow job for this annotation

GitHub Actions / check

Expected '!==' and instead saw '!='
originalSourceURL = sourceURL;
position = pos;
position = { ...pos, line: pos.line + lineOffset };
}
});
if (position.line === null) {
Expand Down
1 change: 1 addition & 0 deletions packages/vscode-extension/src/debugging/DebugSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export class DebugSession implements Disposable {
websocketAddress: websocketAddress,
absoluteProjectPath: getAppRootFolder(),
projectPathAlias: this.metro.isUsingNewDebugger ? "/[metro-project]" : undefined,
expoPreludeLineCount: this.metro.expoPreludeLineCount,
},
{
suppressDebugStatusbar: true,
Expand Down
10 changes: 10 additions & 0 deletions packages/vscode-extension/src/project/metro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ type MetroEvent =
transformedFileCount: number;
totalFileCount: number;
}
| { type: "RNIDE_expo_env_prelude_lines"; lineCount: number }
| {
type: "RNIDE_initialize_done";
port: number;
Expand All @@ -66,6 +67,7 @@ export class Metro implements Disposable {
private _port = 0;
private startPromise: Promise<void> | undefined;
private usesNewDebugger?: Boolean;
private _expoPreludeLineCount = 0;

constructor(private readonly devtools: Devtools, private readonly delegate: MetroDelegate) {}

Expand All @@ -80,6 +82,10 @@ export class Metro implements Disposable {
return this._port;
}

public get expoPreludeLineCount() {
return this._expoPreludeLineCount;
}

public dispose() {
this.subprocess?.kill(9);
}
Expand Down Expand Up @@ -219,6 +225,10 @@ export class Metro implements Disposable {
}

switch (event.type) {
case "RNIDE_expo_env_prelude_lines":
this._expoPreludeLineCount = event.lineCount;
Logger.debug("Expo prelude line offset was set to: ", this._expoPreludeLineCount);
break;
case "RNIDE_initialize_done":
this._port = event.port;
Logger.info(`Metro started on port ${this._port}`);
Expand Down
2 changes: 1 addition & 1 deletion packages/vscode-extension/src/utilities/reactNative.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import path from "path";
import { getAppRootFolder } from "./extensionContext";

export async function getReactNativeVersion() {
export function getReactNativeVersion() {
const workspacePath = getAppRootFolder();
const reactNativeRoot = path.dirname(require.resolve("react-native", { paths: [workspacePath] }));
const packageJsonPath = path.join(reactNativeRoot, "package.json");
Expand Down

0 comments on commit 1023dc7

Please sign in to comment.