Switch to pnpm

[benmccann]

The main benefit is that it allows you to have a monorepo with sub-projects. E.g. right now the main package.json has all the dependencies in it, but in this PR I was able to move the documentation dependencies to the docs sub-project, which is cleaner and helps make it clearer what is used where. Also, this completely removes the need for test/integration/integration-test.cjs and creating a temporary directory for integration testing as we can use workspace dependencies.

As a final benefit, pnpm is much faster than npm especially for subsequent installs of the Chart.js project. The lockfile is also easier to read - you can see the deletion of 28,000 lines. I've been seeing it adopted quite a lot in large projects like Next.js, Vue, Vite, and SvelteKit.

This requires that you install pnpm to build the project. Just run npm install -g pnpm

The compressed size action is failing because it's checking out the previous version from github where pnpm fails because this project is not currently specifying the required peer dependencies. I fixed that in this PR, so all subsequent PRs after this one should pass that check if this is merged.

[LeeLenaleee]

Looks fine only I think it would be best to put a mention in the migration guide for this too.
Guess best place is like in the V3 migration guide in the Developer migration section, small bit of text that we switched from npm to pnpm so that needs to be installed before you can work with hte project again

[benmccann]

Developer migration in v3 was for developers of plugins, etc. However, there won't be any migration required for either end users or developers of plugins. Only people working on this repository itself. That will start immediately with this being merged and will not coincide with the release of v4. So I think putting it in the contributor's guide is probably best.

[LeeLenaleee]

Aah yeah makes sense, my bad

[kurkle]

Faster speed is a good enough reason to switch already. I have some questions though.

[etimberg]

I can replicate the CI failure locally

❯ pnpm test-ci-integration

> chart.js@4.0.0-dev test-ci-integration /mnt/c/Users/evert/opensource/chartjs
> pnpm --filter './test/integration/**' test

Scope: 2 of 4 workspace projects
test/integration/node test$ npm run test-mjs && npm run test-cjs
│ > test-mjs
│ > node test.js
│ > test-cjs
│ > node test.cjs
└─ Done in 536ms
test/integration/react-browser test$ react-scripts build
│ Creating an optimized production build...
│ Failed to compile.
│ [eslint]
│ src/AppAuto.tsx
│   Line 3:9:   'merge' is defined but never used  no-unused-vars
│   Line 8:13:  Trailing spaces not allowed        no-trailing-spaces
│   Line 12:5:  Do not use 'new' for side effects  no-new
│ Search for the keywords to learn more about each error.
└─ Failed in 52s
/mnt/c/Users/evert/opensource/chartjs/test/integration/react-browser:
 ERR_PNPM_RECURSIVE_RUN_FIRST_FAIL  @ test: `react-scripts build`
Exit status 1
 ELIFECYCLE  Command failed with exit code 1.

[LeeLenaleee]

No new should be without putting it in a variable if I'm not mistaken so const c = new Chart() should work although it's an unused car so we might need an update statement or ignore it

[benmccann]

Oops. Should be fixed now. I think it was working originally, but I lost some of the fixes while merging in the latest changes from master

[kurkle]

looks good expect for @LeeLenaleee comments (integration tests for helpers)

[kurkle]

Some actions seem to fail still

[benmccann]

@kurkle looks like you caught it right before I pushed a fix 😄 I had a bad merge when rebasing against master. Should be addressed now. (Except for the Compressed Size one which will start working after this is merged. See the PR description about that one)