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

build: allow using alternate inspector_protocol path #57116

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

codebytere
Copy link
Member

Allow for embedder customization along the lines of similar dependency variables.

Electron needs this in order to prevent conflicts between different implementations in Node.js and Chromium.

@codebytere codebytere requested a review from zcbenz February 18, 2025 09:29
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Feb 18, 2025
@codebytere codebytere added the embedding Issues and PRs related to embedding Node.js in another project. label Feb 18, 2025
@codebytere codebytere force-pushed the allow-inspector-protocol-path branch from 6dd605b to e85451d Compare February 18, 2025 09:31
@codebytere codebytere force-pushed the allow-inspector-protocol-path branch from e85451d to 12634f6 Compare February 18, 2025 09:32
@@ -193,7 +193,7 @@ template("node_gn_build") {
}
if (node_enable_inspector) {
deps += [
"src/inspector:crdtp",
"$node_inspector_protocol_path:crdtp",
Copy link
Contributor

Choose a reason for hiding this comment

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

The crdtp target in inspector_protocol can not be built with Node.js (it has dependency on Chromium's things).

The ideal fix would be removing the dependency on Chromium in the upstream inspector_protocol, pulling it in Node, and then updating here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. embedding Issues and PRs related to embedding Node.js in another project. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants