neovim / node-client

Nvim Node.js client and plugin host
https://neovim.io/node-client/
MIT License
464 stars 49 forks source link

Add name and version to package.json #365

Closed fidgetingbits closed 4 weeks ago

fidgetingbits commented 1 month ago

I'm trying to build node-client on nix using buildNpmPackages, as nix is migrating away from the old way of building node packages, and I get the following:

error: builder for '/nix/store/yp8wvva7jzd0z1p6myafvc7h6j2baiax-neovim-node-client-5.1.0.drv' failed with exit code 1;
       last 25 log lines:
       >
       >
       > > @neovim/integration-tests@4.10.1 build
       > > tsc --pretty
       >
       >
       > > @neovim/example-plugin-decorators@4.10.1 build
       > > babel src --out-dir lib
       >
       > Successfully compiled 2 files with Babel (132ms).
       > Finished npmBuildHook
       > Running phase: installPhase
       > Executing npmInstallHook
       > npm ERR! Invalid package, must have name and version

This seems to be solved simply by adding the name/version into package.json, which this PR does. I've confirmed that the it builds/works correctly with this change, so didn't investigate what exact command was causing the issue in the install hook.

justinmk commented 1 month ago

this probably means the npm version step in https://github.com/neovim/node-client?tab=readme-ov-file#release needs to be updated or another step added. please test if the npm version invocation there also updates the version field you have added here

fidgetingbits commented 1 month ago

this probably means the npm version step in https://github.com/neovim/node-client?tab=readme-ov-file#release needs to be updated or another step added. please test if the npm version invocation there also updates the version field you have added here

Thanks. Unfortunately it does not. I've changed the root package.json to match the prerelease version from packages/neovim/package.json now, and also added the two commands to the README as suggested.

Originally I thought of adding something like this:

    "update-version": "npm version patch && npm version -w packages/neovim patch",
    "prerelease-version": "npm version --no-git-tag-version prerelease --preid dev && npm version -w packages/neovim --no-git-tag-version prerelease --preid dev"

to the root package.json scripts, but from a quick glance it doesn't seem like there is a good way to pass multiple/optional arguments to npm scripts, so wouldn't be pleasant if you need to bump the major/minor version.

justinmk commented 4 weeks ago

This seems like a problem with the nix tooling, since the root package.json doesn't really map to an actual package. But we can try this for now. I need to get a deeper understanding of npm "monorepo" conventions.

fidgetingbits commented 4 weeks ago

Ok, appreciate it. Now that this is merged I'll send a PR to nixpkgs for the new package, and I'll mention this issue to see if someone more familar than me with nix node packaging can clarify if this is a nix problem or not, and I'll let you know.