search

stefanopagliari / bibnotes

359 stars 30 forks source link

fix: don't have node as a dep and add a lock file #77

Open tefkah opened 2 years ago

tefkah commented 2 years ago

I could not develop this version on my local PC because it attempted to install node as a dependency, which I have never seen anyone do but apparently it's not that common.

I would advise against this and just use an .nvmrc or something, for one because this fails on some systems (M1 macs) and secondly because mixing even more versions of node does not seem like a good idea to me.

But it's up to you!

I would recommend adding a lock file though. I like yarn, hence that one, but you should commit it to the repo.

Lovely plugin by the way!

stefanopagliari commented 2 years ago

@ThomasFKJorna thank you for this. I'm very much a beginner when it comes to js/typescript so I'm not exactly sure what is the issue you are describing and the solution. Could you please help me to understand what adding the lock file and removing node from package.json does?

tefkah commented 2 years ago

Sure! Sorry for the late response!

The reason why you would add a lock file is to assure everyone who installs the project using yarn or npm install has the same dependencies installed. If you just specify the dependencies in the package.json, you can run into the problem of two packages requiring the same dependency, but need a different version. When this happens, because your package manager downloads a lot of these at the same time (and other factors), there is no guarantee that all the dependencies will try to use the correct version they need, leading to very frustrating installation procedures that are extremely difficult to debug.

Now, this is not that big of a problem here, as there's only a couple of dependencies, but it's good practice.

As for the node version: since everyone who tries to npm install something already has node on their computer, there is little need to include it. Of course, you want to synchronize versions, but this is better done through a tool such as nvm than through manually downloading another binary, as this can add a lot of size when you have a lot of projects on your pc. It's not that bad, but I've never seen anyone else do it.

stefanopagliari commented 2 years ago

@ThomasFKJorna, sorry I did not have a chance to get back to this until now. Thanks for explaining this to me. You make a lot of sense but I have to admit that I'm not really sure what is required to implement the two steps.

If I understand correctly this pull request, you are suggesting I remove "node": "^17.2.0", from package.json? Since your message, I have added a couple of dependencies (a colour picker and an html-to-markdown formatter).

Regarding the installation of nvm, is it enough to download and install it in the folder of the package or am I required to do something else?