Breaking Change regarding the usage of keytar

[alexdima]

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:

ms-azuretools.vscode-cosmosdb/out/src/tree/AttachedAccountsTreeItem.js:35
  this._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:

• Win 64
• Win 32
• macOS
• Linux 64
• Linux 32

[ejizba]

@chrisdias @chrmarti why are we referencing VS Code's node_modules? Why don't we follow the normal pattern where we add it to our package.json and reference it from our extension's node_modules directly? I'm asking you guys since it was added in this PR: #41

cc: @StephenWeatherford

[StephenWeatherford]

Ah, I assumed we were referencing our own. Good question.

[chrmarti]

Keytar is a native module, so you'd have to ship one version per architecture and bitness (currently 5) and these would need to be rebuilt when we update to a new V8 with a new version of Electron. Having keytar built as part of VS Code does that for us.

[alexdima]

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

[ejizba]

@alexandrudima when will this be going into stable?

[alexdima]

@EricJizbaMSFT With the February release - microsoft/vscode#43361

[alexdima]

Thank you!