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

Ensure source.rootPaths being defined when Node is a child session of Chrome #715

Closed
wants to merge 1 commit into from

Conversation

digeff
Copy link
Contributor

@digeff digeff commented Aug 20, 2020

We were having some issues on Node sessions that were a child of a Chrome session because they were started without the rootSession or cwd configured, and so we couldn't configure the source.rootPath so we ended up showing absolute paths for the loaded script files. With this fix, we show relative file paths instead

@digeff digeff requested a review from connor4312 August 20, 2020 22:30
@connor4312
Copy link
Member

When are Node sessions children of Chrome sessions?

@digeff
Copy link
Contributor Author

digeff commented Aug 20, 2020

@connor4312 We use the server launch.json argument to launch a Chrome session that also has a node session

@connor4312
Copy link
Member

Ah, yea. In retrospect that was a bad idea, done since vscode-pwa had that originally, but a lot of the path resolution stuff isn't the same between them...

@digeff
Copy link
Contributor Author

digeff commented Aug 25, 2020

@connor4312 What would you suggest as the best way to handle this scenario? Is there a different feature/parameter we can use?

@connor4312
Copy link
Member

Not totally sure. I was just looking at our data and wondering whether this is something we want to keep. The number of VS Code users using the server is extremely small and, at least in VS Code, this can be accomplished with compound launch configs or prelaunch tasks.

Thoughts? Do you guys have many people using this feature?

@digeff
Copy link
Contributor Author

digeff commented Aug 27, 2020

@connor4312 We'll try to change our implementation to use two different sessions started independently. Let's hold off on this PR to see if that approach will work without issues.

@digeff digeff closed this Aug 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants