Skip to content

Commit

Permalink
Infra: Switch monorepo tooling to wireit and pnpm (#2345)
Browse files Browse the repository at this point in the history
This PR is a big infrastructure overhaul to switch us from lerna + yarn to wireit + pnpm.

## Why?

Our existing setup of yarn + lerna has the following undesirable things:

1. Installs are slow on yarn
2. Yarn1 isn't updated anymore and simple package dependency updates don't work out of the box. It's a huge pain to update anything.
3. Lerna is getting old, and while the NX team has agreed to support it now, our build is (1) slow, (2) not cached at all, and (3) opaque as to what you need to run task-wise to support other tasks.

So, let's welcome PNPM + Wireit!

1. PNPM is a fast installer and well supported for normal things like package upgrades.
2. Wireit allows fine-grained subtask caching and dependency specifications to allow us to: (1) specify dependencies so when you run `pnpm run jest` all the other things that need to happen magically happen, (2) caches sub-tasks so things that don't have changed file inputs don't re-run, and (3) has full GH actions CI support!

To get a sense of how much faster our build has become take a look at the CI times for this branch in https://github.com/FormidableLabs/victory/actions?query=branch%3Ainfra%2Fpnpm-wireit++ -- when no input files change our CI times are about 1 minute. When some things change, a couple minutes. When all things (or our base scripts) change, we take the full hit of a comparable existing Victory CI time of like 15-16 mins.

For the average Victory developer, if you're working in just one package, you can just run the project-level `pnpm run check` and have like a full build and everything work reasonably fast without needing to know more or run subtasks!

## Check it out

```sh
$ npm install -g pnpm
$ pnpm install
# This will be slow!
$ pnpm run check

# ... but the second time will be super fast! And as you change things subsequent runs should be very fast!
$ pnpm run check
```

## Implementation notes

- High level:
    - **Dependency graph architecture**: All of our tasks now use wireit to identify and run/cache dependencies. So, if you want to run jest, you just run `pnpm run jest` and don't need to worry about "what else needs to be built?" before that.
    - **Parallelization**: To best use wireit, we've refactored our tasks to be more package-specific where possible. E.g., we run lint in incremental mode per-package meaning that both Wireit (at package level) and eslint (at file level within package) re-execute on the narrowest unit of "changed files" possible.

- Demos: The demos originally had imports from `victory*/src/index` (source) which wasn't efficient or consistent because the transitive deps on other victory packages was on built files. I refactored these to be normal `victory(-<NAME>)` imports using built files.

- `src`:
    - Since we did a "fresh" install with pnpm, there were some updates in lint packages that meant source or tests needed small tweaks to continue passing/building.
    - Refactor self-references within a package to not use package name.
    - Fixed missing package dependencies uncovered as pnpm-based installs will fail on missing dependencies that are flattened and "accidentally work" in yarn/npm.

- Configs:
    - Webpack `resolve.alias`: Since we no longer hoist victory packages to root, we now add aliases for our webpack configs and storybook (which uses webpack) programatically.
    - Babel, Jest configs: Normalized the naming and location of these across normal and native ones.

- Incremental caching:
    - eslint
  • Loading branch information
ryan-roemer authored Jul 22, 2022
1 parent 6c16280 commit 1aaa85f
Show file tree
Hide file tree
Showing 109 changed files with 26,989 additions and 19,278 deletions.
File renamed without changes.
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ artifacts
tmp
demo/rn
**/*.d.ts
.wireit
2 changes: 1 addition & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ module.exports = {
},
],
"no-invalid-this": "off",
"@typescript-eslint/no-explicit-any": "off",
"@typescript-eslint/no-invalid-this": ["error"],

"@typescript-eslint/no-non-null-assertion": "off",
"@typescript-eslint/no-unsafe-argument": "off",
"@typescript-eslint/no-unsafe-assignment": "off",
Expand Down
42 changes: 23 additions & 19 deletions .github/workflows/chromatic.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,38 +19,42 @@ jobs:
with:
fetch-depth: 0

- uses: actions/setup-node@v2
- name: Use Node.js
uses: actions/setup-node@v1
with:
# TODO: Upgrade Node version.
# Node18 currently hits `Error: error:0308010C:digital envelope routines::unsupported`
# https://github.com/storybookjs/storybook/issues/16555
# When we upgrade to webpack5 this should go away.
node-version: 16.x

- name: Get Yarn Cache Directory Path
id: cache-dir-path
run: echo "::set-output name=dir::$(yarn cache dir)"
# Wireit cache
- uses: google/wireit@setup-github-actions-caching/v1

- name: Use Yarn Cache
uses: actions/cache@v2
id: cache
- uses: pnpm/action-setup@v2.2.2
with:
path: ${{ steps.cache-dir-path.outputs.dir }}
key: ${{ runner.os }}-yarn-${{ hashFiles('./yarn.lock') }}
version: 7

- name: Set Yarn to Ignore Engines
run: yarn config set ignore-engines true
- name: Get pnpm store directory
id: pnpm-cache
run: echo "::set-output name=pnpm_cache_dir::$(pnpm store path)"

- name: Installation
run: yarn --prefer-offline --frozen-lockfile --non-interactive
- name: Setup pnpm cache
uses: actions/cache@v3
with:
path: ${{ steps.pnpm-cache.outputs.pnpm_cache_dir }}
key: ${{ runner.os }}-pnpm-store-${{ hashFiles('**/pnpm-lock.yaml') }}
restore-keys: |
${{ runner.os }}-pnpm-store-
- name: Build Package Libs
run: yarn nps build-package-libs
- name: Install dependencies
run: pnpm install

- name: Build Storybook
run: yarn build-storybook
run: pnpm run storybook:build

- name: Publish to Chromatic
uses: chromaui/action@v1
with:
token: ${{ secrets.GITHUB_TOKEN }}
projectToken: ${{ secrets.CHROMATIC_PROJECT_TOKEN }}

- name: Remove Yarn Config to Ignore Engines
run: yarn config delete ignore-engines
48 changes: 29 additions & 19 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,31 +28,41 @@ jobs:
node-version: [14.x, 16.x, 18.x]
steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v2

- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v1
with:
node-version: ${{ matrix.node-version }}

- name: Get Yarn Cache Directory Path
id: cache-dir-path
run: echo "::set-output name=dir::$(yarn cache dir)"
# Wireit cache
- uses: google/wireit@setup-github-actions-caching/v1

- name: Use Yarn Cache
uses: actions/cache@v2
id: cache
- uses: pnpm/action-setup@v2.2.2
with:
path: ${{ steps.cache-dir-path.outputs.dir }}
key: ${{ runner.os }}-yarn-${{ matrix.node-version }}-${{ hashFiles('./yarn.lock') }}

- name: Set Yarn to Ignore Engines
run: yarn config set ignore-engines true
version: 7

- name: Installation ${{ matrix.node-version }}
run: yarn --prefer-offline --frozen-lockfile --non-interactive
- name: Get pnpm store directory
id: pnpm-cache
run: echo "::set-output name=pnpm_cache_dir::$(pnpm store path)"

- name: Check Code ${{ matrix.node-version }}
uses: GabrielBB/xvfb-action@v1
- name: Setup pnpm cache
uses: actions/cache@v3
with:
run: yarn nps check.ci
path: ${{ steps.pnpm-cache.outputs.pnpm_cache_dir }}
key: ${{ runner.os }}-pnpm-store-${{ hashFiles('**/pnpm-lock.yaml') }}
restore-keys: |
${{ runner.os }}-pnpm-store-
- name: Install dependencies
run: pnpm install

- name: Remove Yarn Config to Ignore Engines
run: yarn config delete ignore-engines
- name: Check all package.json's and tsconfig.json's are in sync.
run: |
pnpm sync
git diff --no-ext-diff --quiet --exit-code
- name: Build libraries and distributions ${{ matrix.node-version }}
run: pnpm build

- name: Check Code ${{ matrix.node-version }}
run: pnpm check:ci
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ dist
lib
es
node_modules
.pnpm-debug.log*
npm-debug.log*
lerna-debug.log*
yarn-error.log*
Expand All @@ -46,6 +47,10 @@ build-storybook.log*
artifacts
tmp

# Wireit + caches
.wireit
.eslintcache

# Ignore all yarn2+
.yarn/*
.yarnrc*
Expand Down
3 changes: 3 additions & 0 deletions .npmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# TODO(dependencies): Remove this and properly deal with all peerDeps
# https://github.com/FormidableLabs/victory/issues/2236
strict-peer-dependencies=false
3 changes: 3 additions & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ lib-vendor
es
packages/victory-vendor/d3-*
node_modules
.pnpm-debug.log*
npm-debug.log*
lerna-debug.log*
yarn-error.log*
Expand All @@ -15,4 +16,6 @@ tsconfig.json
tsconfig.*.json
storybook-static
public
artifacts
.expo
.wireit
23 changes: 20 additions & 3 deletions .storybook/main.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,36 @@
var path = require("path");
/* globals __dirname:false */
const path = require("path");
const glob = require("glob");
const ROOT = path.resolve(__dirname, "..");
const PKGS = path.resolve(ROOT, "packages");
const STORIES = path.resolve(ROOT, "stories");

module.exports = {
webpackFinal: async (config) => {
// Read all the victory packages and alias.
glob.sync(path.join(PKGS, "victory*/package.json"))
.forEach((pkgPath) => {
const key = path.dirname(path.relative(PKGS, pkgPath));
config.resolve.alias[key] = path.resolve(path.dirname(pkgPath));
});

return config;
},
addons: [
"@storybook/addon-options/register",
{
name: "@storybook/addon-storysource",
options: {
rule: {
test: [/\.stories\.(jsx?|tsx?)$/],
include: [path.resolve(__dirname, "../stories")],
include: [STORIES],
},
loaderOptions: {
prettierConfig: { printWidth: 80, singleQuote: false },
},
},
},
],
stories: ["../**/*.stories.(js|jsx|ts|tsx)"],
// Use glob to locate the stories, because it ignores our circular dependencies.
stories: glob.sync("../**/*.stories.@(js|jsx|ts|tsx)", { cwd: __dirname }),
};
Loading

0 comments on commit 1aaa85f

Please sign in to comment.