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

Tell extensions whether they're running in development mode #95926

Closed
connor4312 opened this issue Apr 22, 2020 · 4 comments
Closed

Tell extensions whether they're running in development mode #95926

connor4312 opened this issue Apr 22, 2020 · 4 comments
Assignees
Labels
api-finalization feature-request Request for new features or functionality notebook verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@connor4312
Copy link
Member

connor4312 commented Apr 22, 2020

Problem

In notebook renderers, there are some special code paths we want to take under development mode. For example, rather than loading a bundle statically, we'd rather load the renderer JavaScript from a webpack-dev-server to take advantage of live reloading / hot module loading. (refs #95908)

The obvious switch we could add for this is an environment variable, but if we add that as a standard variable in a template, then all renderers in the notebook would see the same variable and try to enter development mode.

Proposal

A new property on the extension context, set per-extension if it's running from an --extensionDevelopmentPath or under tests:

enum ExtensionMode {
  Release,
  Development,
  Test
}

export interface ExtensionContext {
  readonly extensionMode: ExtensionMode;
}

We can use this to only enable 'debug mode' for a renderer if that's set, for example:

const bundleUrl = context.extensionMode === ExtensionMode.Development && process.env.WEBPACK_URI
  ? process.env.WEBPACK_URI + '/bundle.js' : path.join(__dirname, 'bundle.js')

Prior Art

The LSP server detects debug mode by seeing if the current process was started with a debug-like flag (ref). However, this comes with the same drawback as using a global environment variable.

If we take the proposed approach, we could adopt the flag as the signal passed to languageserver-node instead of argv detection.

Security Benefit

Extensions could also use this to determine whether to spawn any of their children in debug mode. In miscellaneous places I've seen extensions spawning subprocesses with --inspect so that they can be debugged, but doing so in release can be a vulnerability (#82296). Exposing an standard way to check whether an extension is in development mode might encourage extension authors to be more discreet here.

Limitations

If I'm developing two renders and have multiple extension development paths, this strategy will cause both extensions to be in "debug mode" and could conflict if I use the same/default variables between them.

/cc @rebornix

@connor4312 connor4312 self-assigned this Apr 22, 2020
@rebornix rebornix added this to the April 2020 milestone Apr 22, 2020
@jrieken
Copy link
Member

jrieken commented Apr 23, 2020

I know that there is also requests for knowing if you are in "test running mode" so we might want to combine both proposals. Not yet sure if ExtensionContext is the right place or if this should just be in vscode.env

@connor4312
Copy link
Member Author

connor4312 commented Apr 23, 2020

Something like this?

enum ExtensionMode {
  Release,
  Development,
  Test
}

namespace env {
  export const extensionMode: ExtensionMode;
}

The thought with having it on the context was that the mode is bound to the specific extension, I would have my one single extension in "development" mode but the rest of the extensions in vscode will be in "release" mode.

@jrieken
Copy link
Member

jrieken commented Apr 24, 2020

would have my one single extension in "development" mode but the rest of the extensions in vscode will be in "release" mode.

Is that actually a thing? I mean what's different when developing an extension? Is this just debug mode or does the extension behave different in any way (loaded different etc...) or do we only want a single extension to react to this?

@connor4312
Copy link
Member Author

connor4312 commented Apr 24, 2020

Is this just debug mode or does the extension behave different in any way (loaded different etc...) or do we only want a single extension to react to this?

Yea, that's the idea. In notebooks we'll use that to have only the renderer being developed load its bundle from the webpack dev server rather than the filesystem, while not affecting any other renderers in the notebook.

You could also imagine that if we have a "test" mode, extensions might change their behavior in expectation that test code will provide stubs or mocks. If all extensions were told they were in test mode when trying to test my single extension they may fail in unexpected ways.

@connor4312 connor4312 modified the milestones: April 2020, May 2020 Apr 27, 2020
@rebornix rebornix mentioned this issue May 1, 2020
17 tasks
@connor4312 connor4312 modified the milestones: May 2020, June 2020 May 13, 2020
@jrieken jrieken added feature-request Request for new features or functionality and removed api-proposal labels Jun 9, 2020
@connor4312 connor4312 added verification-needed Verification of issue is requested and removed unreleased Patch has not yet been released in VS Code Insiders labels Jun 15, 2020
@lramos15 lramos15 added the verified Verification succeeded label Jul 1, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jul 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-finalization feature-request Request for new features or functionality notebook verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

5 participants
@rebornix @jrieken @connor4312 @lramos15 and others