scalacenter / scalajs-bundler

https://scalacenter.github.io/scalajs-bundler
Other
235 stars 101 forks source link

Add first-class support for frozen lockfiles #409

Open armanbilge opened 2 years ago

armanbilge commented 2 years ago

(Note: I'm specifically referencing yarn since that's what I'm familiar with, but this issue is relevant for npm as well).

By default, installing npm dependencies with scalajs-bundler+yarn will also possibly change/update versions of transitive npm dependencies (and modify the lockfile yarn.lock). This can cause issues like https://github.com/scalacenter/scalajs-bundler/issues/259. The solution is that yarn should be run with --frozen-lockfile so that the versions in the lockfile are actually respected (i.e., you get a reproducible build), unless you are specifically intending to update your npm dependencies.

In fact, this issue of frozen lockfiles will be directly addressed in the next major version of yarn: https://github.com/yarnpkg/yarn/issues/4147#issuecomment-778251791

IMHO the --frozen-lockfile argument is important enough that it should be exposed as an sbt setting and/or there should be alternative versions of tasks available to install npm dependencies with/without freezing the lockfile. Even better it should be frozen by default. Arguably this is the expected behaviour for those of us used to maven/sonatype (i.e., transitive dependency versions don't randomly change between builds).

This would facilitate integration with plugins such as sbt-github-actions (which have knowledge of whether the build is running in CI).

This would also be extremely helpful in scala-steward so that it is easily able to disable freezing and update a lockfile while updating Scala dependencies. This is critical when changes in npm dependencies are inherited transitively from a Scala dependency. See https://github.com/scala-steward-org/scala-steward/issues/2252.

ptrdom commented 2 years ago

You can achieve this now with yarnExtraArgs setting, they will get passed to yarn install. Implementation of https://github.com/scalacenter/scalajs-bundler/issues/417 will make this capability somewhat more expressive.

ptrdom commented 2 years ago

This PR to be specific - https://github.com/scalacenter/scalajs-bundler/pull/420.

armanbilge commented 2 years ago

Right, this issue is about adding "first-class" support for frozen lockfiles, precisely so you don't have to muck about with yarnExtraArgs. IMHO this is important enough to surface directly to the API. See for example the linked issue about Scala Steward integration.

ptrdom commented 2 years ago

Ah, now I understand the issue. All package managers that I know of (npm, yarn, pnpm) have some means to achieve this. I suppose a setting for frozenLockfile: Boolean would be ideal, defined outside of package manager. Then each package manager should alter its commands to respect it. I would not want to assume what should be the default behavior, package managers themselves are moving towards locking by default. scalajs-bunder is not v1 so we can break things at will 😅 Definitely doable. What do you think? I could work on it.

armanbilge commented 2 years ago

Yes, exactly :) sure, that would be great. However, personally I've become disillusioned about scalajs-bundler and using npm dependencies from Scala.js libs in general (I've focused on replacing them with pure Scala libs instead) so atm I no longer have much interest in this, sorry :(

ptrdom commented 2 years ago

No worries, makes sense to have this feature anyways.

What replacements do you have in mind? Mind sharing some issues/PRs you are working on? I would be interested in checking them out.

armanbilge commented 2 years ago

Here's a few off the top of my head, but if there's something specific you are curious about I can point you in the right direction.