mkYarnPackage: run yarn --offline build in buildPhase? Also, avoid IFD when packageJSON = src + "/package.json"

[doronbehar]

Today I packaged vim-language-server with mkYarnPackage, first time I ever used this Nix function. I noticed after reading the source code and the docs, that there is no buildPhase defined by default, whereas buildNpmPackage runs npm run build by default.

For mkYarnPackage, the default buildPhase I'd expect it to run is:

yarn --offline build

According to a simple git grep survey I did with:

git grep -l -E 'yarn( --offline| run)* build' $(git grep -l -- 'mkYarnPackage' | grep -v yarn2nix/default.nix | grep -v doc/ | grep -v all-packages.nix) | wc -l

There are 37 files using mkYarnPackage. 21 of them are running some form of yarn build, and hence probably could have package these packages with a better experience with that default buildPhase.

ccing people I found with git blame who touched mkYarnPackage before: @happysalada @WilliButz @yu-re-ka

[yu-re-ka]

Don't use mkYarnPackage. It is convoluted and should at some point be replaced by a hook which can be added to any stdenv.mkDerivation package, to allow for multi-language packages too.

[yu-re-ka]

git grep 'yarn.*yarn-offline-mirror' reveals how many packages are already rolling their own thing, and not using the over-complicated mkYarnPackage. Ideally, all these would be de-duplicated once we add a hook.

[doronbehar]

Is there an issue open for implementing mkYarnPackage using hooks?

[yu-re-ka]

I don't think so

[doronbehar]

Even if we change the underlying implementation to use hooks, I think this kind of behavior would be desired.

[doronbehar]

See also: #296857

[yu-re-ka]

I got mkYarnPackage together with yarn2nix merged years ago. It was controversial, and nowadays I see it as a mistake, but it was the best/only tool available at the time.

Now we also have fetchYarnDeps (which is output-compatible with yarn2nix, so it can be used with mkYarnPackage and any successor).
The hooks would be differentiated by yarnConfigHook / yarnBuildHook / ..., just like the npm ones (and for packages which need all hooks there can be a new buildYarnPackage, but I really don't care about that).

[doronbehar]

OK I see. Since you are answering so quickly, do you know the source of the ofborg error I'm getting at: #296847 ? Quoting:

       at /ofborg/checkout/2/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/ofborg-evaluator-1/lib/trivial.nix:308:24:

          307|   importJSON = path:
          308|     builtins.fromJSON (builtins.readFile path);
             |                        ^
          309|

       error: path '/nix/store/a3n603035bc1gwxawljcbjmbk1wjkfcr-source.drv' is not valid

Is it an IFD issue I didn't experience locally? Same error happens in #296850

[yu-re-ka]

Yes, when using mkYarnPackage within nixpkgs, the packageJSON must be set to a in-tree copy of the package.json, otherwise it will attempt to import the package.json from the src to set the package name, version and other inputs, which results in IFD.

[doronbehar]

Hmm interesting, because the fetchYarnDeps does get it's yarnLock from src + "/yarn.lock" - how come that isn't IFD?

[yu-re-ka]

Because this variable is never accessed when yarn.nix is already specified.

misunderstood the question

That's because fetchYarnDeps is not reading the contents of yarnLock, it is merely accessing the path of src/yarn.lock, but the contents are only read in the fetchYarnDeps derivation (not the evaluation).

[doronbehar]

OK I see. Thanks! It still a bit weird because both documentation doesn't mention packageJSON at all, and the default value of this argument is src + "/package.json". So you'd expect stuff would work for standard packages with the default arguments...

It's also a big difference from buildNpmPackage right? buildNpmPackage reads package.json during evaluation in via jq, and not with Nix, which makes it possible for it to avoid IFD right?

[yu-re-ka]

buildNpmPackage does not deal with package.json during eval-time, which also means it can not automatically determine a package version and pname. Basically, for buildNpmPackage the entire npm build tree is invisible to nix (and also npm thinks it's running in a more normal environment), while mkYarnPackage creates lots of small derivations.
The UX is so different because mkYarnPackage was originally made for a very different purpose (which is, being used outside of nixpkgs with as little as possible config, just throwing an mkYarnPackage into a project that already has package.json and yarn.lock in its repo was supposed to "just work", but of course there is no IFD limitations in this environment).

[doronbehar]

Linking here #318015 for case someone reads this.

[doronbehar]

I'm closing this as this is mostly addressed by #318015 .