canva-public / js2nix

Node.js modules installation using Nix
MIT License
63 stars 14 forks source link

Some sort of peer dependency automation #24

Open yangm97 opened 11 months ago

yangm97 commented 11 months ago

I'm trying to adopt js2nix in a react-native app but having to manually resolve each peer dependency in this environment has been... painful.

I very much like the fact js2nix is not opinionated and so guarantees package lookup reproducibility but I wish there was some way to automate the addDependency dance.

olebedev commented 11 months ago

Hi @yangm97,

Yeah, this is a known issue with the current approach. I agree that it's far from perfect and adds maintainability burden which we better to avoid.

As long as the js2nix pretends to be deterministic and reproducible, it acts as a pure function of the yarn.lock file. Unfortunately, the file doesn't provide any information on peer dependencies closure. So we a bit at difficult situation here.

What I can think of is we can try to fetch the whole closure while develop the closure for a project and let js2nix to download the whole closure and infer correct dependency tree. This is essentially what node2nix does and there are quite a lot of complainants about this behavior. TL;DR: it takes ages to re-download stuff on every Node.js closure change. Also, this approach is not reproducible as long as we infer sha sums out of the real artifacts but not rely on what yarn.lock provides, meaning, a package source can simply be changed and within this workflow this error won't be caught.

Another idea to explore is to split the package build into two phases: 1) fetch and unpack, 2) read the peerDependencies and provide it to the final package, if it somehow present in the main Nix expression that was generated by js2nix. I am not sure if this is feasible for us to try to make it work, it would anyways involve the Import From Derivation that we would better to avoid. Let me know what do you think.

The situation you report is basically exactly the same for PNPM. They suggest a couple of valuable approaches here (the option 3 is similar to what we do with addDependency in js2nix).

Note, the peerDependencies option only exists because of non-optimal installation strategies the mainstream package managers for Node.js have, so it's rather a patchwork to make the current imperfectness work but we dig deeper to the rabbit hole by doing that. Js2nix changes that by forcing strict rules that was initially designed for Node.js package management.

Happy to discuss it further to infer the best solution that doesn't contradict to the idea of js2nix to be pure and reproducible. If the IFD approach (which we already have by default in js2nix) is a necessary evil I think we can try to make the second idea work at some extent.

yangm97 commented 11 months ago

I wonder if it would be possible to pay the IFD cost during the yarn.lock.nix generation instead of per package.

The idea would be to download + extract then resolve peerDependencies using versions found on yarn.lock. So js2nix would not add dependencies on its own but would be able to map existing ones deterministically, using the cli. Then building could proceed as usual.

While this is indeed a hack, it seems like way too many babel, react, react native etc. packages intentionally make use of this feature as to avoid duplicate versions of what would be a "global dependency", as metro probably wouldn't like to bundle multiple react versions for a given app. It also kind of works as a way to signal which library versions are compatible with a given react version.

On a side note, I noticed there's a circular dependency cycle going on with babel/core and babel/whatever-transforms, so if I try to override babel/core into this dependency I get an infinite recursion error from the nix side.

The usage guide seems to describe that in these situations packages would be bundled into the same derivation so if that is a bug let me know what I can do to help diagnose this.

olebedev commented 11 months ago

I wonder if it would be possible to pay the IFD cost during the yarn.lock.nix generation instead of per package.

Ah, I see, we can introduce a flag to the js2nix CLI, like --resolve-peer-deps, that forces it to download and infer peer dependencies, this sounds like a valuable option, at least for those who don't want to deal with the manual coding against imperfect dependency tree which a bit of a hassle.

The usage guide seems to describe that in these situations packages would be bundled into the same derivation so if that is a bug let me know what I can do to help diagnose this.

Nope, this is not a bug. The cycles are resolved before the final Nix expression generation while the overrides (addDependency) happen after the final Nix expression generated, so the cycles you introduce after the generation won't be handled by js2nix as described in the documentation.

How do you feel with make a PR that does the trick?

olebedev commented 11 months ago

On a side note, I noticed there's a circular dependency cycle going on with babel/core and babel/whatever-transforms, so if I try to override babel/core into this dependency I get an infinite recursion error from the nix side.

Have a look at the dependency cycles resolution section (I am sure you've done that already, but) you can see that you can manually implement hosted packages as well, using the + character as described, js2nix will be able to recognize that and build the packages hosted as per your manual declaration. In this case you would need to understand the cycles and figure out what would be an optimal host for that cycle. While it's possible technically, I don't think that anyone would want to do that manually, but this is possible.

yangm97 commented 11 months ago

How do you feel with make a PR that does the trick?

Funny thing is the one time I consciously get over the immediate itch to automate something as to not fall into relevant xkcd, it turns out that actually, that would have been the saner option, instead of making this abomination (which doesn't even work so yeah...)

I'm reading the code and it looks like I should run a nix-build in lib/print.js:generateDepTree‎ with a derivation that would simply download the current package, only to get it's package.json, right? Then enhance the currData object with it's peerDepencies. Finally Package.create would need to be updated to take that key into account and it would be golden?

olebedev commented 11 months ago

Oh, yes, this doesn't look neat, 100%.

Yeah, I guess, you would need to modify this section here in the way it asynchronously fetch N packages in parallel and build the tree object so the N is hard-coded but can be changed. I think we can use some kind of library for semaphores in Node.js, not sure if it exists though.

When I was working on this part, I initially made it in async/await manner, but later decided to simplify the code a little.

Please make sure you put this behavior behind an option so the change doesn't affect the current behavior silently. I guess we can also utilize Nix for this task to make sure we cached the "fetch-and-parse-package-json" action in Nix store, so we address the main concern associated with node2nix - re-download the whole closure on any change.