NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
16.59k stars 13.07k forks source link

Repackage node2nix generated packages using buildNpmPackage #229475

Open ehllie opened 1 year ago

ehllie commented 1 year ago

Issue description

Currently a vast majority of NPM packages available in Nixpkgs are generates using node2nix. It generates a nix expression form the node-packages.json file. However it's very hard to maintain. Any time a new package is added to that list or updated, node2nix has to update every single package listed in node-packages.json using a code-gen script. That forces PRs that touch that part of the ecosystem to rerun the code-gen script each time only other PR is merged. This is further exacerbated by the time the script takes to run. As of writing this post, there are 408 packages in node-packages.json, and it takes ~4h to run the generation script.

Proposed solution

Nowadays an alternative tool, buildNpmPackage, is available, but plenty of the packages already listed there were added there before its creation. Not all of the packages can be build using the new tool, but I think it would be worthwhile to review how many of them could be repackaged that way. That should at the very least make the time of running the code-gen script shorter.

As such, I've listed all the packages that are build using node2nix. My intention is this issue could be used to collaborate on the repackaging efforts. If any package gets successfully repackaged, or determined to not be possible to be built using builtNpmPackage, I'd appreciate having that mentioned here so that I can update the list and move that package to an appropriate location.

Not checked - @emacs-eask/cli - @forge/cli - @medable/mdctl-cli - @nerdwallet/shepherd - @shopify/cli - @squoosh/cli - @uppy/companion - @vue/cli - @webassemblyjs/cli - @webassemblyjs/repl - @webassemblyjs/wasm-strip - @webassemblyjs/wasm-text-gen - @webassemblyjs/wast-refmt - alex - alloy - audiosprite - autoprefixer - auto-changelog - aws-azure-login - aws-cdk - awesome-lint - bower - bower2nix - browserify - browser-sync - cdk8s-cli - cdktf-cli - clipboard-cli - coc-clangd - coc-cmake - coc-css - coc-diagnostic - coc-docker - coc-emmet - coc-eslint - coc-explorer - coc-flutter - coc-git - coc-go - coc-haxe - coc-highlight - coc-html - coc-imselect - coc-java - coc-jest - coc-json - coc-lists - coc-ltex - coc-markdownlint - coc-metals - coc-pairs - coc-prettier - coc-pyright - coc-python - coc-r-lsp - coc-rls - coc-rust-analyzer - coc-sh - coc-smartf - coc-snippets - coc-solargraph - coc-spell-checker - coc-stylelint - coc-sumneko-lua - coc-sqlfluff - coc-tabnine - coc-texlab - coc-toml - coc-tslint - coc-tslint-plugin - coc-tsserver - coc-ultisnips - coc-vetur - coc-vimlsp - coc-vimtex - coc-wxml - coc-yaml - coc-yank - code-theme-converter - coinmon - concurrently - cpy-cli - create-cycle-app - create-react-native-app - csslint - dat - dhcp - diagnostic-languageserver - diff2html-cli - elasticdump - @electron-forge/cli - eas-cli - elm-oracle - emoj - emojione - epgstation - epgstation-client - escape-string-regexp - esy - expo-cli - fast-cli - fauna-shell - fixjson - fkill-cli - fleek-cli - forever - fx - ganache - gatsby-cli - generator-code - get-graphql-schema - git-run - git-ssb - git-standup - @gitbeaker/cli - gitmoji-cli - gramma - graphql - graphql-cli - graphql-language-service-cli - grunt-cli - makam - meshcommander - gulp - gulp-cli - he - hs-airdrop - hs-client - ijavascript - inliner - imapnotify - indium - insect - intelephense - ionic - joplin - js-beautify - js-yaml - jsdoc - jshint - json - json-diff - json-refs - json-server - jsonlint - jsonplaceholder - kaput-cli - katex - keyoxide - lcov-result-merger - leetcode-cli - lerna - less - less-plugin-clean-css - live-server - livedown - localtunnel - lodash - lua-fmt - lv_font_conv - madoko - manta - markdown-link-check - mastodon-bot - mathjax - meat - @mermaid-js/mermaid-cli - mocha - multi-file-swagger - near-cli - neovim - nijs - node-gyp - node-gyp-build - node-inspector - node-pre-gyp - node-red - node2nix - nodemon - np - npm-merge-driver - nrm - ocaml-language-server - orval - parcel-bundler - parcel - parsoid - patch-package - peerflix - peerflix-server - pkg - pm2 - poor-mans-t-sql-formatter-cli - postcss - postcss-cli - prebuild-install - prettier_d_slim - prettier-plugin-toml - prisma - @prisma/language-server - pscid - pulp - purescript-language-server - purescript-psa - purs-tidy - purty - pxder - quicktype - react-native-cli - react-tools - redoc-cli - remod-cli - reveal.js - rimraf - rollup - rust-analyzer-build-deps - s3http - sass - semver - serve - serverless - shout - sloc - smartdc - socket.io - speed-test - sql-formatter - ssb-server - stackdriver-statsd-backend - stf - svelte-check - svelte-language-server - svgo - swagger - tedicross - teck-programmer - tern - textlint-rule-alex - thelounge - thelounge-plugin-closepms - thelounge-plugin-giphy - thelounge-plugin-shortcuts - thelounge-theme-abyss - thelounge-theme-amoled - thelounge-theme-amoled-sourcecodepro - thelounge-theme-bdefault - thelounge-theme-bmorning - thelounge-theme-chord - thelounge-theme-classic - thelounge-theme-common - thelounge-theme-crypto - thelounge-theme-discordapp - thelounge-theme-dracula - thelounge-theme-dracula-official - thelounge-theme-flat-blue - thelounge-theme-flat-dark - thelounge-theme-gruvbox - thelounge-theme-hexified - thelounge-theme-ion - thelounge-theme-light - thelounge-theme-midnight - thelounge-theme-mininapse - thelounge-theme-monokai-console - thelounge-theme-mortified - thelounge-theme-neuron-fork - thelounge-theme-new-morning - thelounge-theme-new-morning-compact - thelounge-theme-nologo - thelounge-theme-nord - thelounge-theme-onedark - thelounge-theme-purplenight - thelounge-theme-scoutlink - thelounge-theme-seraphimrp - thelounge-theme-solarized - thelounge-theme-solarized-fork-monospace - thelounge-theme-zenburn - thelounge-theme-zenburn-monospace - thelounge-theme-zenburn-sourcecodepro - tiddlywiki - tsun - ts-node - ttf2eot - uglify-js - undollar - unified-language-server - vega-cli - vega-lite - vim-language-server - vls - vscode-css-languageserver-bin - vscode-html-languageserver-bin - vscode-json-languageserver - vscode-json-languageserver-bin - vue-cli - vue-language-server - wavedrom-cli - webpack - webpack-cli - webpack-dev-server - copy-webpack-plugin - webtorrent-cli - @withgraphite/graphite-cli - wring - @yaegassy/coc-nginx - yalc
Repackage candidate (has package-lock.json) - @tailwindcss/language-server - @tailwindcss/line-clamp - @tailwindcss/typography - create-react-app - npm - pyright - tailwindcss - textlint - textlint-rule-diacritics - textlint-rule-stop-words - textlint-rule-terminology - wrangler
Repackaged - @antora/cli and @antora/site-generator-default #229512 - @bitwarden/cli #218923 - @nestjs/cli #229692 - @google/clasp #229873 - antennas #246296 - asar #251705 - balanceofsatoshis #246434 - bibtex-tidy #246298 - btc-rpc-explorer #249844 - carbon-now-cli #249877 - carto #249879 - castnow #246303 - clean-css-cli #249985 - clubhouse-cli #249990 - coffeescript #250008 - configurable-http-proxy #250203 - cordova #250052 - degit #250056 - dockerfile-language-server-nodejs #250058 - elm-test #250068 - triton, manta #230158 - eslint_d #234166 - firebase-tools #250072 - flood #245435 - glob #250077 - gqlint #250212 - graphqurl #250208 - gtop #246385 - hsd #250269 - html-minifier #250218 - htmlhint #250264 - http-server #250265 - hueadm #246404 - hyperpotamus #250279 - immich #250280 - jake #250284 - javascript-typescript-langserver #250287 - karma #245953 - markdownlint-cli #246145 - markdownlint-cli2 #250807 - musescore-downloader #250250 - npm-check-updates #250875 - react-static #250684 - readability-cli #237463 - reveal-md #246377 - rtlcss #252265 - snyk #252455 - surge #252460 - stylelint #252461 - terser #252499 - three #252430 - titanium #249808 - typescript # 239189 - ungit #250483 - vsc-leetcode-cli #252441 - vscode-langservers-extracted #234888 - web-ext #250477 - write-good #250460 - yo #250462 - zwave-js-server #230380 - zx #246434
Can't be repackaged Usually a package can't be built with `buildNpmPackage` when it has no `package-lock.json` checked out in its vcs. So that's when it's being built with something other than `npm` or when the upstream maintainers just don't track it with vcs. ##### Built with `yarn` - @angular/cli - @commitlint/cli - @commitlint/config-conventional - bash-language-server - markdown-preview-nvim - prettier - textlint-plugin-latex - textlint-rule-abbr-within-parentheses - textlint-plugin-latex - textlint-rule-en-max-word-count - textlint-rule-max-comma - textlint-rule-no-start-duplicated-conjunction - textlint-rule-period-in-list-item - textlint-rule-unexpanded-acronym - textlint-rule-write-good - yarn - yaml-language-server - typescript-language-server ##### Built with `pnpm` - @antfu/ni - @astrojs/language-server - conventional-changelog-cli - cspell - grammarly-languageserver - pnpm - vercel - @microsoft/rush ##### Built with `npm`, but no package-lock.json in vcs - @tailwindcss/aspect-ratio - @tailwindcss/forms - eslint - textlint-rule-common-misspellings

Last list update: 2023 AUG 31

Want to contribute? Here's a general walkthrough of how you can do that

In case the package does not follow the happy scenario you can mention it here so I can move it to the appropriate group of packages. For an example of a package repackaged this way you can check #229512

I've discussed this idea with @SuperSandro2000 in another PR briefly, but I'd appreciate other people responsible for maintaining the node ecosystem to weigh in as well. @NixOS/node would this be the right way to go about this? Where should the repackaged packages be put? Is doing this worthwhile?

winterqt commented 1 year ago

Feel free to move packages out of the set, this is a net good. Not really sure how/if we want to add aliases for the old names, though. (cc @lilyinstarlight)

As a rule of thumb, more or less anything with a package-lock.json can be packaged, but some are impossible due to npm cursedness.

Feel free to request my review directly on packages, I'm happy to land things quickly and work with you on this :)

natsukium commented 1 year ago

Just the other day, I came across a PR to change from nodePackages to a standalone build. (#228854) In this PR, an alias was added to override.nix to keep the nodePackages name. https://github.com/NixOS/nixpkgs/pull/228854/commits/b4b15b69cb9afe1cd6b57c43ab031b49cbfce971

I would also like to help migrate the package.

winterqt commented 1 year ago

Ah, forgot about the overrides file. That looks good, do that. Again, please request my review on these, I'm more than happy to review and get them landed.

Please do keep in mind that we expect you to maintain a package if you add it, so only migrate packages that you are okay with maintaining and testing.

lilyinstarlight commented 1 year ago

@winterqt Should we make a node-aliases.nix kinda like Python has now (either in top-level or still in the node-packages dir)?

That way they are easy to find and turn into throws later (as for other aliases) and we can just make the node-packages default.nix optionally include it on config.allowAliases

winterqt commented 1 year ago

Great catch, yes, let's do it.

ehllie commented 1 year ago

I've gone over a couple of the packages so far, and it seems to be not as straightforward as I initially thought it would be 😛

natsukium commented 1 year ago

For a start, I have categorized the packages around the ones I use. I know it will be hard to update the list, but please check it out.

Repackage candidate (has package-lock.json) - @google/clasp - @tailwindcss/language-server - @tailwindcss/line-clamp - @tailwindcss/typography - create-react-app - dockerfile-language-server-nodejs - markdownlint-cli - npm - pyright - tailwindcss - textlint - textlint-rule-diacritics - textlint-rule-stop-words - textlint-rule-terminology - wrangler
Repackaged - @bitwarden/cli #218923
Can't be repackaged #### `No package-lock.json` ##### with `yarn` - bash-language-server - markdown-preview-nvim - prettier - textlint-plugin-latex - textlint-rule-abbr-within-parentheses - textlint-plugin-latex - textlint-rule-en-max-word-count - textlint-rule-max-comma - textlint-rule-no-start-duplicated-conjunction - textlint-rule-period-in-list-item - textlint-rule-unexpanded-acronym - textlint-rule-write-good - yarn ##### with `pnpm` - conventional-changelog-cli - cspell - grammarly-languageserver - pnpm - vercel ##### with `npm` but no package-lock.json - @tailwindcss/aspect-ratio - @tailwindcss/forms - eslint - markdownlint-cli2 - textlint-rule-common-misspellings
SuperSandro2000 commented 1 year ago

I think we should start with everything that is easy to get done and has a top-level alias. Maybe that is even enough to get nodePackages into shape and we can save ourselves time fixing the really hard and weird cases.

ehllie commented 1 year ago

@natsukium thanks a lot for compiling these packages. I've edited the original post

lilyinstarlight commented 1 year ago

I've added aliases for nodePackages in #229639

malob commented 1 year ago

Just want to note that a missing package-lock.json in the source isn't necessarily a blocker for using buildNpmPackage.

The package-lock.json file can be generated and committed to nixpkgs. This is what I had to do to add github-copilot-cli: https://github.com/NixOS/nixpkgs/blob/1e237b7c3c0d544a0b1efacf22849a0ff261f7a6/pkgs/tools/misc/github-copilot-cli/default.nix#L14-L16

(There is precedent for this type of approach in nixpkgs with Rust packages.)

Mic92 commented 1 year ago

Can we please NOT add 5000 lines of generated packages.json locks per node package? https://github.com/NixOS/nixpkgs/pull/231615/files Otherwise we would end up with ~19.1M lines of code just for 408 nodes packages. We currently have 2.8M lines of code in nixpkgs. This would significantly multiply the size of nixpkgs. I am strongly against that and would like to kindly ask for a different solution that does not bump the nixpkgs tarball to 100MB and also make it no longer possible to run nixpkgs-review on an average laptop (because of memory consumption)

Mic92 commented 1 year ago

Last time I checked node2nix was doing a lot of the fetching in serial. However one could speed it up by pulling with 32-threads instead of just one (7min instead of 4hours based on my napkin math)

natsukium commented 1 year ago

Indeed, in the example above, it was undesirable to commit the 5,000 lines of package-lock.json. On the other hand, if the upstream keeps the package-lock.json, we don't have to add it ourselves and can benefit from a much smaller number of lines in the node-package.nix.

Also, even if we could fetch in parallel, my laptop (macbook pro) only has eight threads, so it is unlikely to be that fast. (although it is enough compared to 4 hours).

RaitoBezarius commented 1 year ago

Can we get statistics / figures on what it would add to nixpkgs before anything?

winterqt commented 1 year ago

FWIW @Mic92, not every package requires that we add a package-lock.json in-tree, only some do. I'd argue that we should migrate all the ones that don't require us to do that first, and then re-evaluate.

RaitoBezarius commented 1 year ago

Apologies for my confusion, I didn't understand it was about adding only those who had package-lock.json in tree. :+1: for the approach!

Mic92 commented 1 year ago

I am ok with this approach when we do this for packages that provide a package-lock.json

SuperSandro2000 commented 1 year ago

We can also upload the package-lock.json to somewhere with buildNpmPackage.

Last time I checked node2nix was doing a lot of the fetching in serial. However one could speed it up by pulling with 32-threads instead of just one (7min instead of 4hours based on my napkin math)

That doesn't fix that it is broken in other ways and confuses dev and prod dependencies.

ehllie commented 1 year ago

We can also upload the package-lock.json to somewhere with buildNpmPackage.

That's definitely one way to do it, but we'd need to decide on a hosting platform for said lock files. It's probably better not to leave the choice up to the individual contributor as that would possibly make maintenance even harder than it is now, with the upstream fetcher endpoints going offline and needing to be updated. I'm also not sure if we wouldn't run into IFD issues by doing that.

But that seems to be a general issue with monorepo approach that Nixpkgs has. Any user of the repository needs to clone the entire repository regardless of how much of it they will actually use. It's obviously not cloning the binaries, but Nixpkgs is continuously growing, and I don't know if that approach is sustainable long term. However that question is beyond the scope of this issue and should probably be addressed in an RFC.

I feel like sticking to packages with pre-existing package-lock.json files and making node2nix make requests in a concurrent manner like @Mic92 suggested is the best course of action we can take atm.

As a side note, I apologise for starting the issue and then disappearing out of the blue. I had just undergone surgery and didn't have much time in the days leading up to it. I should be able to contribute a bit more now that I'm stuck in a hospital bed however ^^

SuperSandro2000 commented 1 year ago

But that seems to be a general issue with monorepo approach that Nixpkgs has. Any user of the repository needs to clone the entire repository regardless of how much of it they will actually use. It's obviously not cloning the binaries, but Nixpkgs is continuously growing, and I don't know if that approach is sustainable long term. However that question is beyond the scope of this issue and should probably be addressed in an RFC.

One of the biggest offender to repo size is nodePackages because the file is just so big.

and don't worry about having time or not. Life and health is more important.

Lord-Valen commented 1 year ago

With yarn:

With rush (thereby pnpm):

pbsds commented 1 year ago

We can also upload the package-lock.json to somewhere with buildNpmPackage.

Perhaps a separate github repo? We could put each lock file into a separate branch/tag, to avoid downloading unrelated locks. Upon request a bot could fetch a package source, npm install --package-lock-only, and commit it to a new branch, then reply with download url, fetchurl hash for the lock file, and the prefetch-npm-deps hash.

EDIT: or gists, they work too

ehllie commented 1 year ago

We can also upload the package-lock.json to somewhere with buildNpmPackage.

That's definitely one way to do it, but we'd need to decide on a hosting platform for said lock files. It's probably better not to leave the choice up to the individual contributor as that would possibly make maintenance even harder than it is now, with the upstream fetcher endpoints going offline and needing to be updated. I'm also not sure if we wouldn't run into IFD issues by doing that.

But that seems to be a general issue with monorepo approach that Nixpkgs has. Any user of the repository needs to clone the entire repository regardless of how much of it they will actually use. It's obviously not cloning the binaries, but Nixpkgs is continuously growing, and I don't know if that approach is sustainable long term. However that question is beyond the scope of this issue and should probably be addressed in an RFC.

I feel like sticking to packages with pre-existing package-lock.json files and making node2nix make requests in a concurrent manner like @Mic92 suggested is the best course of action we can take atm.

As a side note, I apologise for starting the issue and then disappearing out of the blue. I had just undergone surgery and didn't have much time in the days leading up to it. I should be able to contribute a bit more now that I'm stuck in a hospital bed however ^^

My idea with splitting up nixpkgs, is then we could also just split off any package that needs to have its package-lock.json generated into a separate repository, that way it won't affect the size of the actual nixpkgs vsc tree, and won't be included in the tarball download unless the they are actually being built. The same could be done with the massive package set in node-packages.json for the packages that can't use buildNpmPackage, where each of the packages would get it's own repository with the code-gen script, and would be updated on a per need basis, or periodically with a GitHub action. However, I think that would require nixpkgs to further adopt flakes in general. @winterqt @SuperSandro2000 @lilyinstarlight do you recon that would be worth making a separate issue about? Because I think I's worthwhile continuing to repackage things the way I suggested in this thread, but my new idea would probably affect more of nixpkgs as a whole, and be taking a somewhat different approach.

SuperSandro2000 commented 1 year ago

Relying on flakes for nixpkgs is not something that should happen. Also anything that triggers IFD must be avoided. I think the package-lock.json is in a unique position that it is not read by nix directly, so we can move it to somewhere else. For node-packages.json that does not work. Also anything requiring a RFC should be avoided as it will delay things for multiple months.

where each of the packages would get it's own repository with the code-gen script

That would create an insane amount of unmanageable repositories. We can also stuff things into one extra repo with subdirectories for each package. We could work with sparse checkout to only get the subdirectory we really need. I am not sure if that makes things better in the long run.

stasjok commented 12 months ago

With yarn:

* yaml-language-server

Formatting doesn't work in current version of nodePackages.yaml-language-server. I think this is because prettier was updated to version 3.0.0 recently. It is added to yaml-language-server manually here:

https://github.com/NixOS/nixpkgs/blob/7c70396c1de0d0e588ff226b795d54459b135227/pkgs/development/node-packages/overrides.nix#L609-L615

but upstream uses version 2.6.7. I think the only way to fix it is to repackage it with buildNpmPackage so that it doesn't depend on nodePackages.prettier, but to a locked version.

dotlambda commented 11 months ago

The following packages use their own node-packages.nix and should also be repackaged if possible:

K900 commented 11 months ago

I've had some issues with n8n, a look from someone better versed in the JS abyss would be appreciated.

marsam commented 11 months ago

netlify-cli

I have a wip. It builds, but the test is failing https://github.com/NixOS/nixpkgs/pull/243664

panda2134 commented 11 months ago

243664

What OS/platform are you on? AFAIK the tests are failing on macOS due to the sandboxing mechanism.

dotlambda commented 11 months ago

I've had some issues with n8n, a look from someone better versed in the JS abyss would be appreciated.

Looks like n8n uses pnpm, so we can't use buildNpmPackage for now.

nixos-discourse commented 7 months ago

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/buildnodemodules-the-dumbest-node-to-nix-packaging-tool-yet/35733/6

dotlambda commented 5 months ago

Please review https://github.com/NixOS/nixpkgs/pull/285152 which uses fetchYarnDeps for a package that uses lerna.