Switch JS package manager to pnpm #44167

gzdunek · 2024-07-12T18:33:38Z

Implements RFD 177 - Upgrade JS package manager.
Closes #33356
e counterpart https://github.com/gravitational/teleport.e/pull/4652

Best to review commit-by-commit.

I quickly tested Connect (Windows, macOS and Linux builds) and Web UI (macOS), I didn't see any problems.
Test builds: 17.0.0-dev.gzdunek.14 (17.0.0-dev.gzdunek.13 for macOS artifacts).

I will backport it to v16, on v15 and v14 we will stay with yarn. I will add "packageManager": "[email protected]" to them.

Note

To use pnpm, enable Corepack (if you haven't already done it) with corepack enable.
All commands stayed the same, you only need to change yarn to pnpm, e.g. pnpm start-teleport.
The only important difference is dependencies installation, previously you could type yarn now it's pnpm install (or pnpm i).

Warning

After switching back to a branch that uses Yarn, I recommend running make clean-ui before yarn install, otherwise the dependency installation will fail.

It also removes resolutions that are no longer relevant.

`jest-styled-components` package is no longer global, so `toHaveStyleRule` matchers is not recognized by TS. We use them only in three places, so it's simpler to convert them to `toHaveStyle`.

I don't think anyone uses the debug configuration for electron (and we don't have `start-electron` script for a long time).

gzdunek · 2024-07-12T18:44:31Z

.pnpmfile.cjs

Today I realized that pnpm adds an entry for each workspace to the lockfile, including e/web/teleport 😢 (so we will get a different lockfile when e is not cloned).
The only workaround I've found is to manually modify the lockfile using pnpm hooks. I know it's ugly, but I can't think of anything better.
To simplify this hook, I removed all dependencies from the e/web/teleport/package.json.

I think it's a pretty elegant solution actually.

gzdunek · 2024-07-12T18:46:39Z

Makefile

@@ -1643,15 +1643,15 @@ backport:
 .PHONY: ensure-js-deps
 ensure-js-deps:
 	@if [[ "${WEBASSETS_SKIP_BUILD}" -eq 1 ]]; then mkdir -p webassets/teleport && touch webassets/teleport/index.html; \
-	else yarn install --ignore-scripts; fi


--ignore-scripts was causing Connect on Linux not to have pty-node (I don't know why it worked with yarn).

public-teleport-github-review-bot · 2024-07-12T18:48:31Z

@gzdunek - this PR will require admin approval to merge due to its size. Consider breaking it up into a series smaller changes.

github-actions · 2024-07-12T18:48:32Z

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

ryanclark

👏

README.md

ryanclark · 2024-07-16T13:08:55Z

Should we add yarn.lock to the .gitignore?

# Conflicts: # build.assets/Dockerfile # web/packages/design/package.json # yarn.lock

Makefile

gzdunek · 2024-07-16T14:33:52Z

Should we add yarn.lock to the .gitignore?

I think it's a good idea to prevent it from being reintroduced by accident, added.

jest.config.js

web/.storybook/main.js

web/packages/teleport/src/components/BannerList/Banner.test.tsx

README.md

ravicious · 2024-07-16T14:44:51Z

BUILD_macos.md

-  * `brew install node yarn`
-  * Currently, [`yarn`](https://classic.yarnpkg.com/en/docs/install) (< 2.0.0) is required
+  * `brew install node`
+  * `corepack enable`


Suggested change

* `corepack enable`

* `corepack enable pnpm`

Though pnpm docs say this:

If you installed Node.js using Homebrew, you'll need to install corepack separately:

brew install corepack

This will automatically install pnpm on your system.

Should it just say brew install node corepack then?

Uh that's correct, I just tried a fresh installation :/
I suppose it didn't work that way in the past, because I didn't install corepack with brew.

Changed to brew install node corepack.

web/README.md

web/packages/design/src/Icon/README.md

codingllama · 2024-07-16T16:44:29Z

BUILD_macos.md

-  * `brew install node yarn`
-  * Currently, [`yarn`](https://classic.yarnpkg.com/en/docs/install) (< 2.0.0) is required
+  * `brew install node`
+  * `corepack enable`


Similar to the corepack enable pnpm thread (#44167 (comment)), do you have any opinions/advice on using pnpm to manage the node install?

Ie:

brew install pnpm export PNPM_HOME=~/Library/pnpm pnpm env use --global 20 # or whatever version

https://pnpm.io/cli/env#use

@ravicious @gzdunek

I have never tried this before, but it seems really convenient.
We could even provide a specific version in .npmrc, which would be installed automatically for our project when we run any pnpm command:

#.npmrc use-node-version=20.15.0

grzegorz@mbp teleport % pnpm node -v Fetching Node.js 20.15.0 ... v20.15.0

But I believe this change should also done for CI to have a single way of installing Node.js and the package manager. However, I think it would be too much for this PR, but we can go back to this idea in the future.
What do you think?

# Conflicts: # package.json

gzdunek · 2024-07-19T09:53:26Z

Okay, let's merge this!

public-teleport-github-review-bot · 2024-07-19T10:11:53Z

@gzdunek See the table below for backport results.

Branch	Result
branch/v16	Failed

* Adjust `package.json`s to pnpm * Convert resolutions (pnpm doesn't support `**/` syntax) It also removes resolutions that are no longer relevant. * Convert `yarn.lock` to `pnpm-lock.yaml` with `pnpm import` * Always add `e/web/teleport` workspace to the lockfile * Adjust `jest` to new node_modules structure * Adjust `storybook` to new node_modules structure * Replace `toHaveStyleRule` with `toHaveStyle` `jest-styled-components` package is no longer global, so `toHaveStyleRule` matchers is not recognized by TS. We use them only in three places, so it's simpler to convert them to `toHaveStyle`. * Adjust README files * Adjust GHA workflows * Adjust build assets * Remove remaining `yarn` mentions I don't think anyone uses the debug configuration for electron (and we don't have `start-electron` script for a long time). * `corepack enable` -> `corepack enable pnpm` * Automatically enable pnpm when running `make build/teleport` * Run `pnpm import` again * Run `pnpm dedupe` * Add `yarn.lock` to .gitignore * Link to pnpm installation docs * Add a better comment for `transformIgnorePatterns` * Make pattern for `shared` stories more specific * Change `corepack enable pnpm` in readmes * Run `pnpm import && pnpm dedupe` again (cherry picked from commit 620c0a0)

* Switch JS package manager to pnpm (#44167) * Adjust `package.json`s to pnpm * Convert resolutions (pnpm doesn't support `**/` syntax) It also removes resolutions that are no longer relevant. * Convert `yarn.lock` to `pnpm-lock.yaml` with `pnpm import` * Always add `e/web/teleport` workspace to the lockfile * Adjust `jest` to new node_modules structure * Adjust `storybook` to new node_modules structure * Replace `toHaveStyleRule` with `toHaveStyle` `jest-styled-components` package is no longer global, so `toHaveStyleRule` matchers is not recognized by TS. We use them only in three places, so it's simpler to convert them to `toHaveStyle`. * Adjust README files * Adjust GHA workflows * Adjust build assets * Remove remaining `yarn` mentions I don't think anyone uses the debug configuration for electron (and we don't have `start-electron` script for a long time). * `corepack enable` -> `corepack enable pnpm` * Automatically enable pnpm when running `make build/teleport` * Run `pnpm import` again * Run `pnpm dedupe` * Add `yarn.lock` to .gitignore * Link to pnpm installation docs * Add a better comment for `transformIgnorePatterns` * Make pattern for `shared` stories more specific * Change `corepack enable pnpm` in readmes * Run `pnpm import && pnpm dedupe` again (cherry picked from commit 620c0a0) * Run `pnpm import && pnpm dedupe` * Remove `yarn.lock` * make ensure-js-package-manager: Do not prompt before installing pnpm --------- Co-authored-by: Rafał Cieślak <[email protected]>

gzdunek added 11 commits July 12, 2024 19:59

Adjust package.jsons to pnpm

5b68ffc

Convert resolutions (pnpm doesn't support **/ syntax)

ba4c7db

It also removes resolutions that are no longer relevant.

Convert yarn.lock to pnpm-lock.yaml with pnpm import

786e8cb

Always add e/web/teleport workspace to the lockfile

36a5c81

Adjust jest to new node_modules structure

501f69d

Adjust storybook to new node_modules structure

d0317fd

Replace toHaveStyleRule with toHaveStyle

573ab05

`jest-styled-components` package is no longer global, so `toHaveStyleRule` matchers is not recognized by TS. We use them only in three places, so it's simpler to convert them to `toHaveStyle`.

Adjust README files

1b7570c

Adjust GHA workflows

8fe8df7

Adjust build assets

77aa17f

Remove remaining yarn mentions

0412ded

I don't think anyone uses the debug configuration for electron (and we don't have `start-electron` script for a long time).

gzdunek force-pushed the gzdunek/pnpm branch from 6f54a89 to 0412ded Compare July 12, 2024 18:41

gzdunek commented Jul 12, 2024

View reviewed changes

gzdunek marked this pull request as ready for review July 12, 2024 18:48

gzdunek requested review from klizhentas, russjones, r0mant, zmb3, fheinecke, camscale, tcsc, rosstimothy and codingllama as code owners July 12, 2024 18:48

github-actions bot added size/xl ui labels Jul 12, 2024

github-actions bot requested review from rudream and ryanclark July 12, 2024 18:48

ryanclark approved these changes Jul 15, 2024

View reviewed changes

ravicious reviewed Jul 15, 2024

View reviewed changes

README.md Outdated Show resolved Hide resolved

gzdunek mentioned this pull request Jul 15, 2024

Enable pnpm in buildbox #44214

Merged

gzdunek added 2 commits July 16, 2024 12:38

corepack enable -> corepack enable pnpm

f5b21db

Automatically enable pnpm when running make build/teleport

fe09f71

gzdunek added 3 commits July 16, 2024 16:11

Merge branch 'refs/heads/master' into gzdunek/pnpm

c2b76a9

# Conflicts: # build.assets/Dockerfile # web/packages/design/package.json # yarn.lock

Run pnpm import again

50536f7

Run pnpm dedupe

f2f17e2

ravicious reviewed Jul 16, 2024

View reviewed changes

Makefile Outdated Show resolved Hide resolved

Add yarn.lock to .gitignore

5012524

Link to pnpm installation docs

1440b8f

ravicious approved these changes Jul 16, 2024

View reviewed changes

gzdunek added 3 commits July 16, 2024 17:15

Add a better comment for transformIgnorePatterns

1c4c344

Make pattern for shared stories more specific

4369088

Change corepack enable pnpm in readmes

46a346a

codingllama reviewed Jul 16, 2024

View reviewed changes

gzdunek added 2 commits July 19, 2024 11:24

Merge branch 'refs/heads/master' into gzdunek/pnpm

9181509

# Conflicts: # package.json

Run pnpm import && pnpm dedupe again

d020d8d

gzdunek added this pull request to the merge queue Jul 19, 2024

Merged via the queue into master with commit 620c0a0 Jul 19, 2024
41 checks passed

gzdunek deleted the gzdunek/pnpm branch July 19, 2024 10:10

gzdunek mentioned this pull request Jul 19, 2024

[v16] Switch JS package manager to pnpm #44450

Merged

stefanb mentioned this pull request Aug 29, 2024

teleport 16.2.0 Homebrew/homebrew-core#182786

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch JS package manager to pnpm #44167

Switch JS package manager to pnpm #44167

gzdunek commented Jul 12, 2024 •

edited

Loading

gzdunek Jul 12, 2024

ravicious Jul 16, 2024 •

edited

Loading

gzdunek Jul 12, 2024

public-teleport-github-review-bot bot commented Jul 12, 2024

github-actions bot commented Jul 12, 2024

ryanclark left a comment

ryanclark commented Jul 16, 2024

gzdunek commented Jul 16, 2024

ravicious Jul 16, 2024

ravicious Jul 16, 2024

gzdunek Jul 16, 2024

codingllama Jul 16, 2024

gzdunek Jul 17, 2024

gzdunek commented Jul 19, 2024

public-teleport-github-review-bot bot commented Jul 19, 2024

Switch JS package manager to pnpm #44167

Switch JS package manager to pnpm #44167

Conversation

gzdunek commented Jul 12, 2024 • edited Loading

gzdunek Jul 12, 2024

Choose a reason for hiding this comment

ravicious Jul 16, 2024 • edited Loading

Choose a reason for hiding this comment

gzdunek Jul 12, 2024

Choose a reason for hiding this comment

public-teleport-github-review-bot bot commented Jul 12, 2024

github-actions bot commented Jul 12, 2024

ryanclark left a comment

Choose a reason for hiding this comment

ryanclark commented Jul 16, 2024

gzdunek commented Jul 16, 2024

ravicious Jul 16, 2024

Choose a reason for hiding this comment

ravicious Jul 16, 2024

Choose a reason for hiding this comment

gzdunek Jul 16, 2024

Choose a reason for hiding this comment

codingllama Jul 16, 2024

Choose a reason for hiding this comment

gzdunek Jul 17, 2024

Choose a reason for hiding this comment

gzdunek commented Jul 19, 2024

public-teleport-github-review-bot bot commented Jul 19, 2024

gzdunek commented Jul 12, 2024 •

edited

Loading

ravicious Jul 16, 2024 •

edited

Loading