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 to pnpm #527

Merged
merged 1 commit into from
Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions .github/workflows/pull-request-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,22 @@ jobs:
steps:
- uses: actions/checkout@v3

- uses: pnpm/action-setup@v2
with:
version: 8.6

- uses: actions/setup-node@v3
with:
node-version: 16
cache: 'yarn'
cache: "pnpm"

- name: Monorepo init
run: yarn
- name: Install dependencies
run: pnpm install --frozen-lockfile

- name: Build packages
run: yarn run lerna run build
run: pnpm build

- name: Continuous Integration for all packages
run: yarn run lerna run ci --stream
- name: Test packages
run: pnpm test
env:
CI: true
14 changes: 10 additions & 4 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,24 @@ jobs:
with:
fetch-depth: 0

- uses: pnpm/action-setup@v2
with:
version: 8.6

- uses: actions/setup-node@v3
with:
node-version: 16
cache: 'yarn'
cache: "pnpm"
env:
NPM_TOKEN: ${{ secrets.NPM_TOKEN }}

- name: Install dependencies
run: pnpm install --frozen-lockfile

- name: Create Release
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
run: |
yarn install --frozen-lockfile
yarn run lerna run build
yarn run release
pnpm build
pnpm release
12 changes: 0 additions & 12 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,3 @@ package | source
[@cockroachlabs/design-tokens](https://www.npmjs.com/package/@cockroachlabs/design-tokens) | [`/packages/design-tokens`](https://github.com/cockroachdb/ui/tree/master/packages/design-tokens)
[@cockroachlabs/icons](https://www.npmjs.com/package/@cockroachlabs/icons) | [`/packages/icons`](https://github.com/cockroachdb/ui/tree/master/packages/icons)
[@cockroachlabs/ui-components](https://www.npmjs.com/package/@cockroachlabs/ui-components) | [`/packages/ui-components`](https://github.com/cockroachdb/ui/tree/master/packages/ui-components)

## Development setup
This repo is currently managed with [Lerna](https://lerna.js.org/) (version 5.x) which handles symlinking package dependencies for local developement. Each package has local npm scripts for management (`build`, `test`, etc). Before you begin working in the repo you should run `yarn init` from the root of the project.
Other tasks can be found as npm scripts,

```
---
yarn init - `yarn install && yarn lerna run build` -- Install dependencies and build all packages.
yarn start - `lerna run start --parallel` -- Start all packages in watch mode (this can be quite heavy)
yarn release - `auto shipit` -- Used to publish new versions of packages. (Not required for local development)
---
```
6 changes: 1 addition & 5 deletions lerna.json
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
{
"npmClient": "yarn",
"useWorkspaces": true,
"packages": [
"packages/*"
],
"npmClient": "pnpm",
"version": "independent",
"$schema": "node_modules/lerna/schemas/lerna-schema.json",
"useNx": false,
Expand Down
19 changes: 11 additions & 8 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,15 @@
"name": "root",
"private": true,
"devDependencies": {
"auto": "10.43.0",
"lerna": "5.6.2"
"auto": "11.0.4",
"lerna": "^7.2.0"
},
"scripts": {
"init": "yarn install && yarn lerna run build",
"start": "lerna run start --parallel",
"start": "pnpm run --recursive --parallel start",
"test": "pnpm run --recursive test",
"build": "pnpm run --recursive build",
Comment on lines +9 to +11
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The output for these is actually really nice!

"release": "auto shipit"
},
"resolutions": {},
"workspaces": [
"packages/*"
],
"repository": "cockroachdb/ui",
"author": "Cockroach Labs <[email protected]>",
"auto": {
Expand Down Expand Up @@ -48,5 +45,11 @@
"changelogTitle": "📝 Documentation"
}
]
},
"pnpm": {
"overrides": {
"@types/react-lines-ellipsis>@types/react": "17",
"@types/recharts>@types/react": "17"
}
}
}
4 changes: 2 additions & 2 deletions packages/design-tokens/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ Uses [style-dictionary](https://github.com/amzn/style-dictionary) to transform t
## Install dependencies

```
yarn
pnpm install
```

## Generate tokens from json

```
npm run build
pnpm run build
```

## Exported Tokens
Expand Down
4 changes: 1 addition & 3 deletions packages/design-tokens/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@
"types": "dist/web/tokens.d.ts",
"scripts": {
"build": "style-dictionary build",
"clean": "style-dictionary clean",
"prepare": "npm run clean && npm run build",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something about the "automatically build after running pnpm install" behavior felt odd, so I removed it 😄

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair. I never could get the right feel for the correct life-cycle script here. I think I had "prepublishOnly" at one point but it may have been deprecated. This becomes a lot less critical with auto-publishing where we don't need lifecycle scripts to help us remember to "build" before we "publish".

"test": "echo \"Error: no test specified\" && exit 1"
"clean": "style-dictionary clean"
},
"license": "MIT",
"devDependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-config/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
},
"scripts": {
"snyk-protect": "snyk-protect",
"prepare": "yarn run snyk-protect"
"prepare": "snyk-protect"
},
"snyk": true,
"gitHead": "f9e7778c841194fa776c5dc45d62fb9caf408f22"
Expand Down
4 changes: 2 additions & 2 deletions packages/icons/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

## How do I add icons?

Icons are added to this package by copying SVG files to the `/svg` folder. If the icon will have it's `fill` attribute changed via a prop, then copy your file into `/svg/customFill` directory. If the icon should not have its color changed, then copy your file into `/svg/preserveFill`. After copying your file into the appropriate directory, update the `/src/index.ts` file to include your generated icon component for export. You may want to run the build locally to resolve any issues (`yarn build`).
Icons are added to this package by copying SVG files to the `/svg` folder. If the icon will have it's `fill` attribute changed via a prop, then copy your file into `/svg/customFill` directory. If the icon should not have its color changed, then copy your file into `/svg/preserveFill`. After copying your file into the appropriate directory, update the `/src/index.ts` file to include your generated icon component for export. You may want to run the build locally to resolve any issues (`pnpm build`).

Some things to consider about the name of your SVG file:
1. Your `.svg` file will need to have a **unique** name in this repo (that is it can't be named the same as any other `.svg` file in the `/svg` directory).
Expand All @@ -19,7 +19,7 @@ This package serves as a store for SVG icons that are converted and published as

Since the components are generated from `.svg` files they are not committed in this repository. The only file that is kept under source control is the [index.ts](https://github.com/cockroachdb/ui/blob/master/packages/icons/src/index.ts). The components are generated to the directory `src/components` by the npm script [`build:generate:*`](https://github.com/cockroachdb/ui/blob/master/packages/icons/package.json). Once the components are generated, they are transpiled and TypeScript declarations are generated by the [TypeScript Compiler](https://www.typescriptlang.org/docs/handbook/compiler-options.html) (npm script [`build:typescript`](https://github.com/cockroachdb/ui/blob/master/packages/icons/package.json)).

To generate the files locally run `npm run build` or `yarn run build`.
To generate the files locally run pnpm run build`.

### Preserving or removing fill attributes

Expand Down
7 changes: 3 additions & 4 deletions packages/icons/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,14 @@
"main": "dist/index.js",
"types": "dist/index.d.ts",
"scripts": {
"build": "npm run build:generate:stripfill && npm run build:generate:preserve && npm run build:typescript",
"build": "run-s build:generate:stripfill build:generate:preserve build:typescript",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooo. What is run-s? Does that come with pnpm? If it's a dependency-less npm-run-all then I love it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately it's an alias that comes wioth npm-run-all. I don't think pnpm supports sequential script targets natively 🤔

"build:typescript": "tsc",
"build:generate:stripfill": "svgr --typescript --icon -d src/components svg/customFill",
"build:generate:preserve": "svgr --typescript --icon --no-runtime-config -d src/components svg/preserveFill",
"build:watch": "tsc --watch",
"build:watch": "tsc --watch --preserveWatchOutput",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this, tsc --watch clears the terminal while other watch tasks don't and it looks weird!

"clean": "rimraf dist src/components",
"prepare": "npm run clean && npm run build",
"test": "node notests.js",
"start": "npm run build:watch"
"start": "pnpm run build:watch"
},
"license": "MIT",
"devDependencies": {
Expand Down
13 changes: 13 additions & 0 deletions packages/storybook-ui-components/.storybook/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,19 @@ module.exports = {
name: "@storybook/react-webpack5",
options: {},
},
async webpackFinal(config, { configType }) {
if (configType === 'DEVELOPMENT') {
// Silence some of the typical webpack spew when running in watch mode,
// so that a top-level 'pnpm start' is still usable.
config.infrastructureLogging = {
...config.infrastructureLogging,
level: "error",
};
}

return config;
},

docs: {
autodocs: "tag",
},
Expand Down
7 changes: 2 additions & 5 deletions packages/storybook-ui-components/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
},
"homepage": "https://github.com/cockroachdb/ui#readme",
"dependencies": {
"@cockroachlabs/icons": "^0.7.6",
"@cockroachlabs/ui-components": "^0.7.7",
"@cockroachlabs/icons": "workspace:../icons",
"@cockroachlabs/ui-components": "workspace:../ui-components",
Comment on lines +21 to +22
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a huge improvement honestly. I haven't gotten there yet, but any idea how this will be handled when building storybook for deployment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It shouuuuuuuld automatically link everything together :)

"ts-loader": "9.4.2",
"typescript": "4.8.4"
},
Expand All @@ -42,8 +42,5 @@
"storybook": "7.0.20",
"ts-loader": "9.4.2",
"webpack": "5.75.0"
},
"resolutions": {
"prismjs": "^1.21.0"
}
}
24 changes: 13 additions & 11 deletions packages/ui-components/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,27 +9,27 @@
"main": "dist/main.js",
"types": "dist/types/index.d.ts",
"scripts": {
"build": "npm run build:typescript && npm run build:bundle",
"build": "npm-run-all build:typescript build:bundle",
"build:bundle": "webpack --mode production",
"build:typescript": "tsc",
"build:watch": "webpack --mode development --watch",
"ci": "jest --ci -i",
"clean": "rm -rf ./dist/*",
"lint": "eslint \"src/**/*.{tsx,ts,js}\"",
"lint:fix": "eslint \"src/**/*.{tsx,ts,js}\" --fix",
"prepare": "npm run clean && npm run build",
"test": "jest --watch",
"test:version": "npm version prerelease --preid='alpha'",
"test:publish": "npm publish --tag testing",
"test": "jest",
"test:watch": "jest --watch",
"test:version": "pnpm version prerelease --preid='alpha'",
"test:publish": "pnpm publish --tag testing",
"publish-test-version": "npm-run-all test:version test:publish",
"start": "npm run build:watch"
"start": "pnpm run build:watch"
},
"keywords": [],
"license": "MIT",
"dependencies": {
"@cockroachlabs/icons": "^0.7.6",
"@cockroachlabs/icons": "workspace:../icons",
"@popperjs/core": "^2.9.2",
"@types/recharts": "^1.8.23",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was never a runtime dependency — moved to dev dependencies

"lodash": "^4.17.21",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accidentally-missing dependency (previously hoisted from another package)

"react-lines-ellipsis": "^0.15.0",
"react-popper": "^2.2.5",
"recharts": "^2.1.12"
Expand All @@ -42,8 +42,8 @@
"@babel/preset-env": "7.20.2",
"@babel/preset-react": "7.18.6",
"@babel/preset-typescript": "7.21.0",
"@cockroachlabs/design-tokens": "^0.4.12",
"@cockroachlabs/eslint-config": "^1.0.5",
"@cockroachlabs/design-tokens": "workspace:../design-tokens",
"@cockroachlabs/eslint-config": "workspace:../eslint-config",
"@testing-library/jest-dom": "5.16.5",
"@testing-library/react": "12.1.5",
"@types/classnames": "2.3.0",
Expand All @@ -52,6 +52,8 @@
"@types/node": "16.18.14",
"@types/react": "17.0.53",
"@types/react-lines-ellipsis": "0.15.1",
"@types/recharts": "^1.8.23",
"@types/testing-library__jest-dom": "^5.14.6",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accidentally-missing dependency (previously provided by @testing-library/jest-dom as a transitive dependency)

"@typescript-eslint/eslint-plugin": "5.54.0",
"@typescript-eslint/parser": "5.54.0",
"babel-loader": "8.3.0",
Expand All @@ -68,6 +70,7 @@
"jest-environment-jsdom": "29.4.3",
"jest-runner-eslint": "1.1.0",
"mini-css-extract-plugin": "^2.7.5",
"npm-run-all": "4.1.5",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accidentally missing dependency (got hoisted from another package)

"prettier": "2.8.4",
"react": "17.0.2",
"react-dom": "17.0.2",
Expand All @@ -86,6 +89,5 @@
"react": "^17.0.2",
"react-dom": "^17.0.2"
},
"resolutions": {},
"gitHead": "a77405daccfaf2d38cc08ef7c322172ee93776fd"
}
6 changes: 0 additions & 6 deletions packages/ui-components/webpack.config.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/* eslint-disable @typescript-eslint/no-var-requires */
/* globals module, __dirname */
const path = require("path");
const WebpackBar = require("webpackbar");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It got in the way of watch task reporting and spewed way too many lines. I can put it back if we want, but 🤷

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah, this was something I was playing with way back in the beginning. It was only something to clean up the output of watching only ui-components. I agree it doesn't make sense in the context of file watching.

const MiniCssExtractPlugin = require("mini-css-extract-plugin");

module.exports = {
Expand Down Expand Up @@ -55,11 +54,6 @@ module.exports = {

plugins: [
new MiniCssExtractPlugin(),
new WebpackBar({
name: "ui-components",
color: "cyan",
profile: true,
}),
],

// When importing a module whose path matches one of the following, just
Expand Down
Loading