Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix misaligned lines in debugger #634

Merged
merged 12 commits into from
Oct 22, 2024
15 changes: 7 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,14 @@ 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", lineOffset: 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 @@ -25,8 +25,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
41 changes: 34 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 @@
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 @@
private absoluteProjectPath: string;
private projectPathAlias?: string;
private threads: Array<Thread> = [];
private sourceMaps: Array<[string, string, SourceMapConsumer]> = [];

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

Expand All @@ -96,6 +98,7 @@
this.absoluteProjectPath = configuration.absoluteProjectPath;
this.projectPathAlias = configuration.projectPathAlias;
this.connection = new WebSocket(configuration.websocketAddress);
this.lineOffset = configuration.lineOffset;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This parameter should really be called expoPreludeExtraLines or something along these lines instead of being so generic


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,31 @@
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]);

// This is a heuristic that checks if the source map should contain __env__
// module that is added by expo, but not reported in the source map
kmagiera marked this conversation as resolved.
Show resolved Hide resolved
const isFileWithOffset = sourceMap.sources.includes("__prelude__");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename isMainBundle or isFullBundle


// This line is here because of a problem with sourcemaps for expo projects,
// that was addressed in this PR https://github.com/expo/expo/pull/29463,
// unfortunately it still requires changes to metro that were attempted here
// https://github.com/facebook/metro/pull/1284 we should monitor the situation
// in upcoming versions and if the changes are still not added bump the version below.
kmagiera marked this conversation as resolved.
Show resolved Hide resolved
const shouldApplyOffset =
kmagiera marked this conversation as resolved.
Show resolved Hide resolved
semver.lte(getReactNativeVersion(), "0.76.0") && isFileWithOffset;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we check for expo version here and not react native?

if (this.lineOffset !== 0 && shouldApplyOffset) {
Logger.debug(
"Expo prelude lines were detected and an offset was set to:",
this.lineOffset
);
}

this.sourceMaps.push([
message.params.url,
message.params.scriptId,
consumer,
shouldApplyOffset ? this.lineOffset : 0,
kmagiera marked this conversation as resolved.
Show resolved Hide resolved
]);
this.updateBreakpointsInSource(message.params.url, consumer);
}

Expand Down Expand Up @@ -264,7 +291,7 @@
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,16 +300,16 @@
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 306 in packages/vscode-extension/src/debugging/DebugAdapter.ts

View workflow job for this annotation

GitHub Actions / check

Expected '!==' and instead saw '!='
sourceURL = pos.source;
}
if (pos.line != null) {

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

View workflow job for this annotation

GitHub Actions / check

Expected '!==' and instead saw '!='
sourceLine1Based = pos.line;
}
if (pos.column != null) {

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

View workflow job for this annotation

GitHub Actions / check

Expected '!==' and instead saw '!='
sourceColumn0Based = pos.column;
}
}
Expand Down Expand Up @@ -440,7 +467,7 @@
}
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 @@ -451,9 +478,9 @@
column: columnNumber0Based,
bias: SourceMapConsumer.LEAST_UPPER_BOUND,
});
if (pos.line != null) {

Check warning on line 481 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,
lineOffset: this.metro.expoPreludeLineOffset,
},
{
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 @@ -8,7 +8,7 @@
import { shouldUseExpoCLI } from "../utilities/expoCli";
import { Devtools } from "./devtools";
import { getLaunchConfiguration } from "../utilities/launchConfiguration";
import WebSocket from "ws";

Check warning on line 11 in packages/vscode-extension/src/project/metro.ts

View workflow job for this annotation

GitHub Actions / check

`ws` import should occur before import of `../utilities/subprocess`

export interface MetroDelegate {
onBundleError(): void;
Expand Down Expand Up @@ -46,6 +46,7 @@
transformedFileCount: number;
totalFileCount: number;
}
| { type: "RNIDE_expo_env_prelude_lines"; lineOffset: number }
| {
type: "RNIDE_initialize_done";
port: number;
Expand All @@ -66,6 +67,7 @@
private _port = 0;
private startPromise: Promise<void> | undefined;
private usesNewDebugger?: Boolean;
private _expoPreludeLineOffset: number = 0;

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

Expand All @@ -80,6 +82,10 @@
return this._port;
}

public get expoPreludeLineOffset() {
return this._expoPreludeLineOffset;
}

public dispose() {
this.subprocess?.kill(9);
}
Expand Down Expand Up @@ -207,6 +213,10 @@
}

switch (event.type) {
case "RNIDE_expo_env_prelude_lines":
this._expoPreludeLineOffset = event.lineOffset;
Logger.debug("Expo prelude line offset was set to: ", this._expoPreludeLineOffset);
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
Loading