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

Breaking Change regarding the usage of keytar and semver #217

Closed
alexdima opened this issue Feb 19, 2018 · 1 comment
Closed

Breaking Change regarding the usage of keytar and semver #217

alexdima opened this issue Feb 19, 2018 · 1 comment

Comments

@alexdima
Copy link
Member

Hi,

I'm working on VSCode and I am trying to enable the usage of ASAR for VSCode. This can improve the startup time of VSCode by up to 5%. We are at a point where we are pursuing even such small gains...

I have found that your extension makes an assumption about the disk layout of VSCode's node_modules:

PeterJausovec.vscode-docker/out/commands/system-prune.js:19
  const semver = require(`${vscode.env.appRoot}/node_modules/semver`);
PeterJausovec.vscode-docker/out/explorer/models/dockerHubUtils.js:19
  const keytar = require(`${vscode.env.appRoot}/node_modules/keytar`);
PeterJausovec.vscode-docker/out/explorer/models/registryRootNode.js:27
  this._keytar = require(`${vscode.env.appRoot}/node_modules/keytar`);
PeterJausovec.vscode-docker/out/explorer/utils/dockerHubUtils.js:19
  const keytar = require(`${vscode.env.appRoot}/node_modules/keytar`);

To make your extension entirely compatible with a distribution of VSCode that uses ASAR, and to make your extension work with both VSCode stable and VSCode insiders, you will need to try to load from two paths... Something like the following:

function getNodeModule(moduleName) {
  try {
    return require(`${vscode.env.appRoot}/node_modules.asar/${moduleName}`);
  } catch(err) { }
  try {
    return require(`${vscode.env.appRoot}/node_modules/${moduleName}`);
  } catch(err) {}
  return null;
}

I plan to release the changes to Insiders this week, such that the Insiders build on Friday, 23.02.2018, will contain the ASAR packing of node modules. In the meantime, I have created custom builds that include this change if you wish to test your extension against a VSCode version which contains these changes:

@alexdima
Copy link
Member Author

I have submitted a PR to make the adoption - #220

@vscodebot vscodebot bot locked and limited conversation to collaborators Feb 18, 2019
Knas2121 pushed a commit to Knas2121/vscode-docker that referenced this issue Sep 7, 2021
Fixes microsoft#217: Adopt the usage of ASAR in VSCode
Knas2121 added a commit to Knas2121/vscode-docker that referenced this issue Sep 7, 2021
Fixes microsoft#217: Adopt the usage of ASAR in VSCode
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant