Released JupyterLab uses different dependencies than the master yarn.lock specifies

[jasongrout]

Recently there was a bug in @lumino/commands version 1.18.0 which prevented arrow shortcut keys from working (see jupyterlab/lumino#255, jupyterlab/lumino#151, and jupyterlab/lumino#258; jlab issue at #11412).

It turns out that @lumino/commands 1.15.0 is what yarn.lock is locked to (this is the root yarn.lock file at the 3.2.2 release commit):

jupyterlab/yarn.lock

Lines 2352 to 2353 in 4852e50

	"@lumino/commands@^1.12.0", "@lumino/commands@^1.15.0":
	version "1.15.0"

However, something in the release process actually released JLab 3.2.2 using @lumino/commands 1.18.0 (this is the yarn.lock file in our 3.2.2 release staging directory at the same commit):

jupyterlab/jupyterlab/staging/yarn.lock

Lines 1731 to 1732 in 4852e50

	"@lumino/commands@^1.12.0", "@lumino/commands@^1.18.0":
	version "1.18.0"

The end result is that we are releasing something that we are not seeing or testing in our dev branches. That concerns me, in that we can pick up regressions in dependencies that we won't catch in our normal dev workflow, like we did for 3.2.2.

[jtpio]

Thanks Jason for catching this 👍

It looks like @lumino/commands was updated in this commit for 3.2.2: 4852e50. But many of the previous release commits also seem to be updating staging/yarn.lock so maybe that's been the case for a while already?

So probably something to do with the logic in update-core-mode.ts?

jupyterlab/buildutils/src/update-core-mode.ts

Lines 60 to 61 in d860b89

	// Create a new yarn.lock file to ensure it is correct.
	utils.run('jlpm', { cwd: staging });

[jasongrout]

Here is where we create a new yarn.lock in the release process:

jupyterlab/buildutils/src/update-core-mode.ts

Lines 60 to 67 in d860b89

	// Create a new yarn.lock file to ensure it is correct.
	utils.run('jlpm', { cwd: staging });
	try {
	utils.run('jlpm yarn-deduplicate -s fewer --fail', { cwd: staging });
	} catch {
	// re-run install if we deduped packages!
	utils.run('jlpm', { cwd: staging });
	}

[jasongrout]

Here is the commit where we started creating a fresh new yarn.lock file in the release process: 90693b8

This was during the 0.30 days years ago. @blink1073 - do you remember what problem we were trying to solve by creating a new yarn.lock rather than using something like the current yarn.lock? I remember discussing it, but I don't remember exactly the problem we were running up against.

[blink1073]

Best I can remember is that the dependencies are different between the two top level packages, but I'm not sure if we considered allowing yarn to prune the unnecessary packages.

[jasongrout]

do you remember what problem we were trying to solve by creating a new yarn.lock

Actually, looking at that commit where it was introduced (90693b8), we see that it was taking care of a duplication in the yarn.lock. It may be that we were simply trying to deal with yarn having a conservative update policy that easily introduces duplicates. These days, we also run the yarn-deduplicate script just after creating a new yarn.lock, so that original concern might be resolved anyway.

Best I can remember is that the dependencies are different between the two top level packages, but I'm not sure if we considered allowing yarn to prune the unnecessary packages.

Good point about the master yarn.lock containing a superset of the packages we need. I just tried copying the root yarn.lock to the staging directory and running jlpm, and it did indeed prune a lot of packages.

[krassowski]

I will just note that this caused another regression (in sanitizer! this could have gone very wrong) and made it difficult to debug because it was actually a dependency of sanitize-html that got updated and broke it (see #11473 (comment)) - and it was impossible to reproduce in dev mode/on binder because of this issue. Depending on how long we plan to support 3.2 (and on whether there is going to be 33.3), backporting the fix to 3.x might be useful for our sanity.