keytar brings in a lot of files in the build

[aeschli]

I'm looking into reducing the number of files in our server build.

Turns out that keytar brings in a lot of file because the use of prebuild-install in the install script.
Because of that prebuild-install is a dependency (not just a dev dependency) and prebuild-install and all it's dependencies end up in the bits we ship (tar-fs, tar-stream, readable-stream....).

It's not easy to remove all these unnecessary from the build. We have a hand curated filter (https://github.com/microsoft/vscode/blob/main/build/.moduleignore) but that's cumbersome and might change after an update.

[JacksonKearl]

tar-fs/etc. isn't me, seems to be a node module that keytar depends on: https://www.npmjs.com/package/tar-fs.

[aeschli]

I think we need to solve this in our build scripts. In

vscode/build/lib/dependencies.ts

Line 60 in b0cbac3

function getYarnProductionDependencies(cwd: string): Dependency[] {

we compute the tree of dependencies. There we could remove the keytar dependencies (along with all it's subdependencies) as we know they are not needed at runtime.

I want to look at implementing this, unless there are better ideas. @joaomoreno ?

[joaomoreno]

prebuild-install is a production dependency. It needs to be, since npm doesn't have the notion of installation-time dependencies. Unfortunately, it brings many other things: https://www.npmjs.com/package/prebuild-install?activeTab=dependencies

There we could remove the keytar dependencies (along with all it's subdependencies) as we know they are not needed at runtime.

I'm not sure you'll find success here since dependencies might be shared with another one of our dependencies, due to deduplication.

Given that keytar doesn't have any dependencies other than installation-time dependencies, we can take the approach of installing keytar on an isolated location, and then just copying the keytar folder to our node_modules, excluding everything in keytar/node_modules. This also has the risk of breaking as soon as keytar depends on something, but feels like a smaller risk to me.

Another approach: fork keytar into a version which ships all prebuilt binaries variants at install time and have a postinstall step to delete all which do not apply to the desired os/arch.

[TylerLeonhardt]

Those both seem like decent options. The former seems easier...

@deepak1556 do you have any advice here?

Also is there something I could ask the keytar folks to do that would make this easier?

[deepak1556]

Agree with @joaomoreno on the available solutions, sqlite3 native dependency also has the same issue and we took the second approach with a slight variant microsoft/vscode-node-sqlite3@59a82b2. We simply do not rely on the prebuilt binaries in the fork, since essentially we want to ship native modules for both our client and server compiled with the toolchain we have customized for our CI based on the electron and node versions, prebuild is unnecessary in these cases.

I don't think the upstream keytar can make things easier for us, since prebuild-install cannot be made optional. The issue we are facing is specific to how we bundle and ship these modules, so I would align with a fork and control what we want to ship.

[TylerLeonhardt]

@deepak1556 do you think we could meet and go over the approach with vscode-node-sqlite3 and how I could do the same for keytar? I like the idea of following that pattern and I'm curious to know how we publish vscode-node-sqlite3

[thegecko]

I don't think the upstream keytar can make things easier for us, since prebuild-install cannot be made optional

Came across the same issue using this package in a VS Code extension, recommend supporting keytar moving to using prebuildify

atom/node-keytar#255

[TylerLeonhardt]

Looks like my issue should be marked as a dupe :) atom/node-keytar#462

@thegecko btw, I noticed you mentioned bringing in keytar for a vscode extension... you actually don't need to bundle keytar in your extension because we shim keytar so that you use the keytar that ships in vscode.

In your code, you can use keytar as you do today (importing it as normal), but you don't need to bundle it (and it's deps) with your extension.

[thegecko]

In your code, you can use keytar as you do today (importing it as normal), but you don't need to bundle it (and it's deps) with your extension.

That's awesome! is it easy to discover other packages which can be leveraged in this way? Especially native ones.

[TylerLeonhardt]

@thegecko that comes from here:

vscode/src/vs/workbench/api/common/extHostRequireInterceptor.ts

Line 36 in 3268d1d

export abstract class RequireInterceptor {

It's really just keytar, the vscode module itself, obviously, and the open/opn module.

Keytar behaves this way so that you have consistency if your extension is actually running on a remote. You get the value stored on the client side (the side with an actual keychain).