Use yarn-deduplicate instead of cleaning

[vidartf]

Speed up lab build by using yarn-deduplicate instead of cleaning out yarn lock every build.

Question: There were previously two cleaning steps involved, the pre_clean and the explicit removal of a yarn lock file in staging. The PR changes the default of pre_clean to False, and removes the yarn lock clean step. While the yarn lock step should now be safe, some care should be taken to ensure that changing the pre_clean default doesn't surface any other issues.

[vidartf]

Note on performance: Quick informal tests locally on my Windows machine indicate that this change brings the yarn time on rebuilds down from 30-50 s to 1-6 s. The webpack build time is unaffected.

[vidartf]

PS: This should be doubled checked against previous issues: #4543 and #3827.

[bollwyvl]

from 30-50 s to 1-6 s.

Hooray faster, more reproducible build!

The webpack build time is unaffected

There's probably more we can do here, if we're optimizing for build time. the runtime chunk pattern would save even looking at chunks that hadn't changed: https://webpack.js.org/configuration/optimization/#optimization-runtimechunk Of course, "vendors main" kitchen sink is the long pole, and almost guaranteed to be changed by The Next Extension You Install, so it would likely yield minimal benefits over what we could have... Indeed, moving phosphor, react, services (basically shell) into their own bundle might make the greatest impact on build time. More chunks mean more requests, but slower-changing chunks mean better caching. Might have a very positive impact, but it's hard to say without numbers. Of course the big kahuna of caching, service worker, remains unexplored... I recall getting it to work back on the 0.27ish time frame, but it didn't make a lot of sense in my use case. But given some of the cool things out there, it might make sense long term: https://github.com/edoardocavazza/unchained Finally, even though I have made no progress (sorry) on a general pattern for offlining the whole extension mechanism, there could be the argument for distributing a fully-baked build environment (webpack and loaders) in another yarn.lock (and another package!) through Another Package Manager, such that the only thing that actually changed on build was the extensions the user added. Last I checked, the yarn cache of tarballs for the tool chain was about 15mb, but there's still the odd random multi megabyte binary download (ugh) not covered by that. Such a package, Jupyterlab-webpack could then carry the nodejs dependency (in conda), and/or be allowed to version at a different pace than the user-interesting piece. And for the pypi crowd, there's still... https://github.com/sqreen/PyMiniRacer To explore, in case we really wanted ANOTHER another v8 runtime hanging around (I think I have about 6).

…

[vidartf]

@bollwyvl There's definitely more than can be done for improving build times (I'm exploring some things for webpack), and I'd be happy to discuss that in more detail. That said, if you want to write up your current thoughts in a new issue, I think that would be a lot cleaner. I don't want this PR to become an omnibus for all optimizations ;)

[vidartf]

@blink1073 The CI failures are relevant: It fails when trying to do yarn install in <root>\node_modules\@jupyterlab\application-top, as there is no yarn.lock there. Any clues about what is going on? 😕

[blink1073]

Hmm, the issue is that there is no yarn.lock in the dev_mode folder. I think we instead need to insert the post install script as part of this script: https://github.com/jupyterlab/jupyterlab/blob/master/buildutils/src/update-core-mode.ts

[vidartf]

@blink1073 I've just gotten back from a break. From what I understand, you are saying that:

• yarn-deduplicate yarn.lock should be added to the update-core-mode script. (between jlpm and jlpm run build:prod ?)
• The postinstall script should be removed from the dev_mode and/or staging package.json file(s). If so, which one(s)?

[vidartf]

Or would it maybe be enough to remove this line?

[blink1073]

Yeah, I think removing that line should be sufficient.

[vidartf]

After having thought a bit more about this, I think the current pattern should be a lot better. Again, this should be tested manually against some known previous problem case.

[jasongrout]

I think the current pattern should be a lot better

Do you mean this PR should be better than the current codebase, or that the current codebase should be better than this PR?

[vidartf]

I mean, the current design in this PR is better than the first attempt made in this PR.

[blink1073]

@vidartf, I don't know of a good way to replicate the original error we saw. I say we try this in a pre-release in the wild and see if any problems arise. The PR needs a rebase though...

[jasongrout]

Adding [WIP] since it needs a rebase. Whoever does a rebase, please remove [WIP] from the title when done.

[vidartf]

All green 🍀

[blink1073]

Thanks!