Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch from yarn to npm; repair dependency graph #2576

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

jake-low
Copy link
Contributor

@jake-low jake-low commented Feb 21, 2025

What

This PR does the following:

  • removes yarn.lock, add package-lock.json
  • upgrades (and in one case downgrades) dependencies to achieve compatibility between peer dependencies (for example, packages that have a peerDependency on react are all compatible with React 17.0.2 now, whereas before some only claimed to be compatible with React <= 16 or React >= 18).
  • deletes a couple of unused dependencies
  • refactors one widget (the Featured Challenges carousel) in order to drop our dependency on the unmaintained react-elastic-carousel package.

How did we get into a situation where the versions of our direct dependencies were incompatible with each other? Because of Yarn's dependency resolution strategy, which is looser and less predictable than you would hope. More details below.

Why

MapRoulette uses Yarn 1.x, which has been in maintenance mode since Jan 2020. Yarn 1.x uses dependency resolution strategy that is different from npm's, and leads to behavior that library authors do not expect, which ultimately means that many @types packages (containing Typescript type definitions) cannot be installed correctly. This is a blocker for any incremental adoption of Typescript in the MapRoulette codebase. It has also resulted in our dependency graph growing more and more corrupted over time - specifically, Yarn did not enforce that packages' peer dependency ranges were compatible with one another. Yarn 1.x also has numerous other unfixed issues (1, 2, 3).

Yarn 2.x and above are radically different from Yarn 1.x. Modern versions of Yarn use a module installation strategy called Plug'n'Play in which there is no node_modules folder. Instead, Node, Typescript, Vite, and any other programs that load modules must be monkeypatched to resolve them through Yarn's .pnp.cjs file. Frankly I think this decision is bizarre. Lots of tooling that we take for granted has reimplemented Node's node_modules resolution algorithm (from VSCode to typescript's language server to Github's dependabot) and throwing this away creates a ton of headaches.

Yarn was originally created to address shortcomings in npm at the time: namely poor performance, no support for lockfiles, and limited support for workspaces (monorepos containing many JS packages which depend on each other; a common strategy for large libraries like Lodash or Turf). But npm today has a local package cache (improving install speeds), and support for lockfiles and workspaces. It also has a new, much more sophisticated module resolution algorithm called Arborist.

In short, there is no longer a compelling reason to stick with Yarn, and upgrading to a modern version of Yarn looks unappealing. The better choice IMO is to instead switch back to npm.

Todo

This PR is still a draft because there are a few issues left to deal with:

  • Drop react-table v6, which is incompatible with React 17. This is unfortunately a pain, because react-table v7 (which is compatible with React 17) is a total rewrite. We use this dependency in a few different views; I think it's probably best to drop the dependency altogether and implement the functionality we need ourselves.
  • Update the docs to replace references to yarn with npm (e.g. in instructions about setting up the dev environment).
  • Fix CI scripts (which currently expect to use yarn install etc).

@jake-low jake-low mentioned this pull request Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant