rhino
rhino copied to clipboard
Replace `yarn` with `npm`
It's good to have as little dependencies as possible. Alternatively we could at least warn user that yarn
is not available and what can be done about this.
This change should be easy to introduce in the future (in terms of backwards-compatibility). Potential drawback should be considered though: perhaps yarn
works much faster than npm
, or maybe there is some other good reason to prefer it.
I tried doing this too,
yarn
vs npm
Here are my changes: https://github.com/Appsilon/rhino/compare/main...feat/change-yarn-to-npm
The shift from yarn
to npm
seemed to be straightforward. The syntax was different as npm
required that extra run
command.
The main issue for me was setting the current working directory. For yarn
it was --cwd
, for npm
, I learned it was --prefix
. Correct me if I am wrong, as this is my first time playing around with yarn
and npm
.
Running the scripts also required an extra --
when setting arguments for the commands, e.g. lint-js
when fix = TRUE
had to be run as lint-js -- --fix
.
npm install
error
The problem here though is npm_install()
.
As I have noted, it is not working yet. By default, when I run npm install
in terminal or via system2()
it tries to find package.json
in root. It throws an error because it should use package.json
in .rhino/node
.
I tried using --prefix
and --nodedir
but it still tries to find package.json
in root.
Trying npm install .rhino/node
returns this error
npm ERR! Cannot read properties of undefined (reading 'spec')
The only way I was able to rebuild node_modules
and create package-lock.json
is by running
cd .rhino/node && npm install
I did this in terminal and had to cd ../..
later to return to root, which is quite impractical.
I also don't know how to do this via system2()
.
Dependency Warnings
I am also getting this errors when rebuilding modules. Maybe we need some updates on the dependencies?
npm WARN deprecated @stylelint/[email protected]: Use the original unforked package instead: postcss-markdown
npm WARN deprecated [email protected]: The querystring API is considered Legacy. new code should use the URLSearchParams API instead.
Thanks @vibalre for experimenting with this! It's possible that --prefix
won't do and we'll actually need to change the working directory whenever we run npm
(perhaps using withr::with_dir()
.