Investigate moving from yarn back to npm

[joaomoreno]

When we first moved to Yarn, it was mostly a performance move. Nowadays npm has mostly caught up and Yarn went a completely different direction. We're sitting on Yarn v1, mostly abandoned by the project leads. We should investigate the possibility of moving back to npm.

[joaomoreno]

I'm currently hitting this on Windows and it's blocking me: npm/cli#4028

https://stackoverflow.com/questions/66893199/hanging-stuck-reifyprettier-timing-reifynodenode-modules-nrwl-workspace-comp

This is sad.

[kkocdko]

Glad to see this change! The yarn v1 was rarely used now, back to NPM is good for new contributors and the health of the vscode's future.

And I'm inquisitive if there is a plan to drop the gulp? The latest version of gulp is v4.0.2
on May 7, 2019. It's abandoned and it needs to write many build scripts, which is inconvenient compare to modern bundlers.

[jasonwilliams]

Hey @joaomoreno how is this going?
As a contributor its annoying having to install yarn v1 just for this project, when most other repo's don't need it.

Is that npm bug still a problem? How can we repoduce it to see if its still causing the issue?

[starball5]

I'm curious to know whether using pnpm has been considered. I've been a pretty happy user of it. The space savings with links is quite good. Some starter info can be found at https://pnpm.io/motivation. Also, sorry if I've already asked this elsewhere. My brain thinks it vivdly remembers having brought this up, but can't remember where or when, and my attempt to search for previous comments of mine mentioning pnpm haven't turned that up.

[oenu]

I'm curious to know whether using pnpm has been considered. I've been a pretty happy user of it. The space savings with links is quite good. Some starter info can be found at https://pnpm.io/motivation. Also, sorry if I've already asked this elsewhere. My brain thinks it vivdly remembers having brought this up, but can't remember where or when, and my attempt to search for previous comments of mine mentioning pnpm haven't turned that up.

Seconded, if work is to be done to migrate should pnpm not be considered?

[deepak1556]

This milestone we have resumed exploration on this issue, https://github.com/microsoft/vscode/tree/robo/yarn_2_npm tracks this effort and we have our first unreleased insiders build from it 🚀 . There were some breaking changes and/or differences that require documenting as follows,

1. Peer dependencies
   Unlike yarn 1.x, npm starting with v7 installs peer dependencies by default. We have a couple of conflicting peer dependencies that required the use of --legacy-peer-deps flag. If we pursue further down the npm path it would be good to eventually remove this flag so that we can use flags like --prefer-dedupe that reduces duplication in the bundle.
   a) xterm and its addons - We rely on beta versions of the @xterm/xterm package while the addons have a peer dependency on stable versions. @Tyriar
   b) typescript and ts-node - Same as xterm our workspace relies on dev version of typescript that does not satisfy the peer dependency of ts-node @mjbvz
   c) @microsoft/applicationinsights-* and tslib - Telemetry package has peer dependency on tslib: "*" that conflicts with tslib from root. @lramos15

2. .npmrc
   We have 3 different .yarnrc today,
   a) .yarnrc - In the project root, that configures Electron related settings for building native node modules of the client
   b) remote/.yarnrc - Configures remote server specific Node.js settings
   c) build/.yarnrc - Configures build related packages to use system installation of Node.js
   We start installation from project root via yarn and using a postinstall script we navigate sub folders like build/, remote, extensions/ to perform module installation in each of them relying on the .yarnrc files described above. With npm we still maintain .npmrc for each of those folders so that a developer can do things like,

> cd remote
> npm ci -> will use the .npmrc from `remote/.npmrc`

However when running npm ci from project root only the configuration from root will take effect, we workaround this by relying on the priority order of npm configurations and setting env variables in the postinstall script for each of those sub directories. Npm has a workspace feature, should check if it can used to improve this installation flow.

3. Private registry auth
   We use custom feed for our product builds and setup authentication with --location=project which yarn 1.x applies to all sub folders even in the postinstall step however npm does not. We can workaround this by generating the auth settings for the global .npmrc configuration.

4. vsce
   vscode-vsce package helps with generating the file list that needs to be used for extension bundling, it relies on the package manager of choice to also go through the dependency tree. When switching from vsce.packageManager.Yarn -> vsce.packageManager.Npm the extension bundle size doubled due to node_modules being included when they were not with yarn. Investigating this a bit more, there seems to be a bug with the vsce package where the dependency logic is different with yarn vs npm specifically the part about checking PackedDependencies. Packed dependencies are dependencies declared in package.json which are also declared as externals in webpack configs. A quick static check confirms there are no external dependencies that requires packing today, so as a workaround I have switched the package manager to use vsce.packageManager.None which results in the same bundle tree with npm as it was with yarn. We should have this addressed in vsce eventually. @jrieken who originally added this support in Support for 'vscode:packagedDependencies' vscode-vsce#282, can you confirm if adding similar support to getNpmDependencies the way to go forward ?

[lramos15]

c) @microsoft/applicationinsights-* and tslib - Telemetry package has peer dependency on tslib: "*" that conflicts with tslib from root. @lramos15

This can be ignored, the package doesn't even really use Tslib except for super old backwards compatibility which does not apply to our case. See internal issue

[Tyriar]

a) xterm and its addons - We rely on beta versions of the @xterm/xterm package while the addons have a peer dependency on stable versions. @Tyriar

@deepak1556 to fix this would I need to specify the beta version of @xterm/xterm in beta addon releases?

[deepak1556]

@Tyriar yeah that would help. Just to note, comparator operators on prerelease tags strictly match on the [major, minor, patch] tuple https://docs.npmjs.com/cli/v6/using-npm/semver#prerelease-tags so you might need to declare more ranges on your peer deps depending on how your prereleases satisfy. But looking at the xterm releases this might not be an issue.

^5.4.0-beta : >=5.4.0-beta <6.0.0 only matches beta.x within 5.4.0, 5.4.1-beta.x or 5.5.0-beta.x will not satisfy

[deepak1556]

@lramos15 thanks I will create a separate issue to track that. Given that issue is open for a while I think we need to contribute to removing that peer dependency if it is of serving no value today in upstream. We cannot ignore it, we can only workaround it by using the --legacy-peer-deps flags which I am doing today. The goal here is to eventually get rid of this flag.

[lramos15]

We will need to update packages but should be fixed via microsoft/ApplicationInsights-JS#2397

[deepak1556]

Rollout Plan

09/05 JST morning - Flip the pipeline to disallow dependency changes from incoming PR
09/05 - 09/06 - Remind team to avoid merging PR that has dependency changes
09/05 EOD PST - Update and run unreleased insiders build from robo/yarn_2_npm branch
09/06 CET morning - Merge PRs in vscode, vscode-distro and vscode-build-tools after the scheduled insiders is released
09/06 CET morning - Run insiders from main
09/06 CET morning - Send post merge announcement

[Tyriar]

The xterm peerDependency problem should be fixed with #227313