jashkenas / backbone

Give your JS App some Backbone with Models, Views, Collections, and Events
http://backbonejs.org
MIT License
28.1k stars 5.39k forks source link

Upgrade devDependencies, add lockfile #4246

Closed jgonggrijp closed 2 years ago

jgonggrijp commented 2 years ago

Since there was no lockfile yet, I decided to pull out yarn. Yarn gave me lots of warnings, so it's clearly time for some upgrades.

ogonkov commented 2 years ago

Is yarn really necessary?

jgonggrijp commented 2 years ago

No, of course every other package manager will work as well. Would you prefer another one, and if so, which and why?

ogonkov commented 2 years ago

I guess npm is good enough in current state. An im one of this people, that believe that lock file is not required for library.

jgonggrijp commented 2 years ago

Now we seem to be discussing two topics.

Whether to have a lockfile or not: as far as I know, all current package managers (at the very least npm and yarn) give you a lockfile by default, creating it on the spot if it didn't exist yet. So the question is only whether to commit it or not. Given that every single person who installs Backbone's devDependencies is going to have a lockfile anyway, we might as well all have the same one, no? In fact, I think that is one of the main purposes of a lockfile: ensuring that everyone on the team is working on the exact same thing.

Which package manager to use: sure, npm is good enough, but so is yarn. On top of that, yarn is faster and deterministic and it deduplicates dependencies. I find it convenient. I can be convinced to use another package manager, but the argument will have to be stronger than "good enough".

ogonkov commented 2 years ago

package-lock.json will not be packaged. If we commit it, that would be for our own CI/developer team, user will end with different package-lock (that the reason why i never use it in my libs).

ogonkov commented 2 years ago

I'm not pushing here to avoid package-lock, just opinion

jgonggrijp commented 2 years ago

I appreciate your opinion, and your active participation in general. Please continue sharing your opinion.

Yes, the lockfile is for our team and for CI. I consider that enough reason to commit it. Let's put that part of the discussion aside, because I believe there is really no added value in keeping it out of Git.

Regarding whether to use npm or yarn, I'm still leaning towards yarn, but I'll allow some time for more people to chime in with opinions and arguments. I'm not in a hurry.

paulfalgout commented 2 years ago

I typically prefer npm to yarn, but as @jgonggrijp is the most active contributor at this point, I'd say dealers choice. However I do think keeping backbone and underscore in similar tooling is ideal, and it looks like underscore is using npm? Is it switching to yarn?

jgonggrijp commented 2 years ago

Why do you typically prefer npm over yarn, @paulfalgout? I would be really interested to learn that.

You are right that Underscore is still using npm. I abstained from the transition there, because it already had a package-lock.json. I might eventually switch Underscore to yarn as well, though.

I like the consistency argument, but that consistency has already been lost. Or it is already leaning towards yarn, depending on how you look at it. Underscore-contrib and Underscore Fusion (which I still need to publish) are using yarn already. So is another library I am currently working towards publishing, which I expect to be somewhat "associated" with Underscore and Backbone as well.

paulfalgout commented 2 years ago

Nothing overly particular re: yarn. I think at one point yarn was clearly ahead of npm. Then at one point once npm improved I migrated just about everything from yarn and gulp etc etc etc to npm and webpack or rollup to reduce tooling dependencies. Most of my repos have an .nvmrc so anyone running nvm will already have the correct (or close enough) version of npm for that version of node and nothing additional to install. Yarn is just 1 additional complexity.. typically with everything.. most CIs require some additional yarn step etc.. but I no longer see a particular benefit to that complexity.

All that said, it's not that big of a deal to support yarn, nor is it hard to switch back. ATM you seem to be the foremost contributor to the backbone-ecosystem so whatever your most comfortable with seems best.

jgonggrijp commented 2 years ago

Generally, I am much more interested in making a good choice than in making a comfortable choice. I appreciate your kind gesture, though.

You wrote that npm had improved, which @ogonkov had already hinted at as well. I suspected I had probably missed something about npm's recent developments, so I investigated. I ended up looking at this, this, this, this and this. I learned a couple of things:

For a moment I was tempted to suggest adding both a yarn.lock and a package-lock.json and letting contributors decide for themselves whether to use yarn or npm 7+. I rejected the idea, because I realized that the scripts in the package.json assume a particular package manager. It would also somewhat neutralize the purpose of having everyone on the team work on exactly the same environment.

I think I'll compare the combined size of the node_modules and the lockfile with and without the --flat/--prefer-dedupe flag. If no clear winner comes out of that comparison, I'll go with npm v7, on the grounds that the scripts already assume npm.

jgonggrijp commented 2 years ago

Most of my repos have an .nvmrc so anyone running nvm will already have the correct (or close enough) version of npm for that version of node and nothing additional to install.

Maybe Backbone should do that too? Could you give an example of how that works?

Yarn is just 1 additional complexity.. typically with everything.. most CIs require some additional yarn step etc.. but I no longer see a particular benefit to that complexity.

In yarn's defense, the Node.js images from Docker Hub, GH Actions, GitLab, Travis etcetera nowadays all seem to include yarn by default, so this additional complexity seems to have been eliminated.

paulfalgout commented 2 years ago

.nvmrc is just the node version. So most of my projects are just that file containing 14 or 16 stored in the root. Then you can run nvm use for a project. I have a vscode plugin that just runs it automatically when I open the terminal for a project. It doesn't enforce it, but it's a help.

Essentially the work of up-keeping node is keeping CI and the .nvmrc files up to date.

It's more of a tool like .editorconfig that saves steps for devs that use the plugins.

Yeah yarn is typically installed on docker instances.. however for some projects I'm on, the docker image for CI is php and we then install node.. at that point you're always having to also install yarn... again.. negligible, but I didn't see any benefit from yarn any more.

jgonggrijp commented 2 years ago

I compared yarn classic with and without the --flat option and npm v7 with and without the --prefer-dedupe option. I took the following steps for each variant:

  1. Clone the repo and cd into it.
  2. Run installation from scratch without lockfile (not timed because this is not something that will be done frequently and because I wanted to give both tools the opportunity to cache everything first).
  3. Run install again while everything is already installed, time this (first column in the table below).
  4. Delete the node_modules but not the lockfile.
  5. Run install again (always without the --flat or --prefer-dedupe option), time this (second column in the table below).
  6. Take the final size of the lockfile and the node_modules (final two columns).

I skipped the flattening/deduping option in steps 3 and 5, because we can't realistically expect contributors to remember to include that option every time, and the lockfile should theoretically maintain the previously established deduplication.

pacman nothing tbd reinstall lockfile size node_modules
npm 2.46 s 12.9 s 348 kB 71.6 MB
npm --prefer-dedupe 2.39 s 13.4 s 348 kB 71.6 MB
yarn 0.76 s 12.4 s 127 kB 71.3 MB
yarn --flat 0.78 s 11.8 s 111 kB 61.7 MB

What I learned:

Overall, yarn seems a bit better for the environment while npm seems to be more correct. Those qualities are both important so I'm torn. Fortunately, I have two tie breakers in favor of npm: (1) the scripts in the package.json which currently assume npm and (2) the existing preference for npm of @ogonkov and @paulfalgout. So npm it will be.

Adding a yarn.lock as well, just for faster and smaller installation in CI, might be an option. Thoughts?

Slightly off-topic: based on the above comparison, there doesn't seem to be enough reason to switch to npm in projects where I'm already using yarn. However, I'm not sure what to choose in future projects. If anyone could offer me a killer argument to help me decide, that would be great.

jgonggrijp commented 2 years ago

Adding a yarn.lock as well, just for faster and smaller installation in CI, might be an option. Thoughts?

Update: for comparison, I also tried running yarn in the npm clone. Yarn disregards the contents of the package-lock.json and starts recursively resolving all dependencies all over again, even if all dependencies have already been installed, making slightly different choices from npm. I then went back to the yarn clone where I had also run npm install, and ran yarn over there as well. Yarn did the same thing, despite the fact that there was already a yarn.lock (which npm had updated). The result is that yarn will not be faster than npm in CI, unless we commit to using yarn only.

Concluding, dual lockfiles has no clear advantage, rather a disadvantage. This experiment also reinforces my impression that npm is more correct overall. Just a package-lock.json it will be.

Rayraz commented 2 years ago

While I'm not qualified to make any technical judgement here, honestly you had me at 'npm seems to be more correct'. Assuming that assessment isn't wrong, I'd say that's as close to a 'killer argument' as it gets when things are otherwise pretty much the same?

Thanks for doing a deep dive on this, this is why I enjoy lurking in these repos. Always interesting things to learn.

jgonggrijp commented 2 years ago

Yes, you're right. "More correct" by itself is already a very strong argument. Minimizing energy usage is important, too, though. Then again, the fact that yarn spends less time on the wall clock is only evidence, not proof, that it consumes less energy overall.

Regarding the energy department, I contend myself for now with the knowledge that I can pass --no-audit to npm install.

Out of interest @Rayraz: did you have a preference between npm and yarn before reading this thread? If so, what was it based on?

Rayraz commented 2 years ago

I didn't have a strong opinion one way or another, but I feel like I encounter npm more frequently. I tend to use npm on my own projects, but that's really just a force of habit. I can't recall having had problems with either one any time in the past few years to be honest.

Having said that, the more people use a project, the more I feel like an effort should be made to make sure things 'just work' and npm being 'more correct' would play into that nicely.

When it comes to energy usage, I feel like it's kind of insane how much code even a simple build environment uses these days. If we frequently need a hundred megabytes of code to generate just a few kilobytes worth of production code that just seems to me like something's going pretty badly wrong. If both npm and yarn end up producing pretty much the same results, that's probably a sign the bigger problem is located somewhere else.

jgonggrijp commented 2 years ago

@Rayraz

Having said that, the more people use a project, the more I feel like an effort should be made to make sure things 'just work' ...

I suspect causation works mostly in the opposite direction, i.e., effort to make sure things "just work" tends to positively influence adoption, all other things being equal. Then again, since npm has been around for longer, the maintainers have had more time to figure out all the things that need to work. I'm thinking this might be a case of "old is gold".

... and npm being 'more correct' would play into that nicely.

Yes, I see how that makes sense.

When it comes to energy usage, I feel like it's kind of insane how much code even a simple build environment uses these days. If we frequently need a hundred megabytes of code to generate just a few kilobytes worth of production code that just seems to me like something's going pretty badly wrong. If both npm and yarn end up producing pretty much the same results, that's probably a sign the bigger problem is located somewhere else.

Yes, it is kind of insane if you think about it. I know from experience that Python virtualenvs tend to grow to similar sizes as a typical node_modules, and the same is probably true of most other programming languages and packaging systems nowadays. I think this effect has two driving forces: (1) the fact that we keep stacking new abstractions on top of previous abstractions and (2) the fact that we are increasingly trying to isolate things from each other (think: program modules, separate node_modules for each project, containers, and so forth). I think both of those effects are good in principle; (1) lets us focus on building new things instead of reinventing the same things over and over again and (2) improves maintainability and security. Nevertheless, I agree there is clear, big downside to it.

(And, having written that, I realize that I tend to treat "correct" as a stronger argument than "efficient", since I'll happily pull out docker-compose and fire up complex constellations of containers.)

Rayraz commented 2 years ago

I suspect causation works mostly in the opposite direction, i.e., effort to make sure things "just work" tends to positively influence adoption, all other things being equal.

In terms of initial adoption, that's probably true. However, the more people depend on a specific library / tool / whatever, the more damage one can inflict by breaking things. So in that sense I feel like there is an increasing responsibility.

In this case though, npm and yarn are both probably as safe a bet as can reasonably be expected.

jgonggrijp commented 2 years ago

Right. I agree with that.