Skip to content

feat: migrate from yarn to pnpm#567

Merged
hyf0 merged 19 commits into
mainfrom
release-flow
Mar 13, 2024
Merged

feat: migrate from yarn to pnpm#567
hyf0 merged 19 commits into
mainfrom
release-flow

Conversation

@milesj

@milesj milesj commented Mar 12, 2024

Copy link
Copy Markdown
Contributor

Description

Part of #543

This PR does a bunch of things:

  • Migrates from yarn to pnpm. I ran some small benchmarks and pnpm was about 3x faster on cold installs, but the same on warm installs.
yarn 4.1.1 pnpm 8.15.4
Cold 14.42s 4.5s
Warm 1.45s 1.3s
  • Audited dependencies of every package.json, and removed deps that weren't used, or should be in the root.
  • Tested all scripts, and fixed any issues I encountered.

Test Plan


@netlify

netlify Bot commented Mar 12, 2024

Copy link
Copy Markdown

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit c33b66a
🔍 Latest deploy log https://app.netlify.com/sites/rolldown-rs/deploys/65f1a5c26f89720008215ee5

Comment thread package.json Outdated
"scripts"
],
"scripts": {
"nuke": "rm -rf node_modules && rm -rf packages/*/node_modules && rm -rf web/*/node_modules && rm -rf examples/*/node_modules && rm -rf crates/rolldown_binding_wasm/node_modules && rm -rf scripts/node_modules",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

For testing...

Comment thread packages/bench/index.js
const bench = new Bench({ time: 100, iterations: suite.benchIteration ?? 10 })

// Check if inputs have been initialized
for (const input of suite.inputs) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

When testing, this would crash hard if just setup-bench wasn't ran, so added this little check.

"@types/node": "^20.11.25",
"npm-run-all2": "^6.1.2",
"prettier": "^3.2.5",
"colorette": "^2.0.20",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There are used in tests, but weren't listed. Because pnpm scopes them, they're now required.

@@ -1,4 +1,4 @@
import type { RollupOptions, RollupOutput } from 'rolldown'
import type { RollupOptions } from '../../../../src'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Because of pnpm, rolldown was no longer able to reference itself, so had to use relative paths.

Comment thread .github/workflows/ci.yml Outdated
Comment on lines +103 to +107
- name: Install pnpm
run: corepack enable

- name: Install pnpm
run: corepack enable

@Demivan Demivan Mar 13, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is duplicated.
actions/cache needed to cache pnpm packages, so they are not downloaded every time.

@milesj

milesj commented Mar 13, 2024

Copy link
Copy Markdown
Contributor Author

Dependencies install in less than 45 seconds, typically around 15-20. Not really worth adding caching at this point.

@Demivan

Demivan commented Mar 13, 2024

Copy link
Copy Markdown
Contributor

Dependencies install in less than 45 seconds, typically around 15-20. Not really worth adding caching at this point.

Why not make it even faster? It is just 4–5 lines of CI code.

@hyf0

hyf0 commented Mar 13, 2024

Copy link
Copy Markdown
Member

@milesj Do you have time to clean up this PR? So we could merge the migration first. We could solve release problems in another PR.


I saw the c-spell job failed. You could add the issue file to cspell.json#ignorePaths to ignore it.

@hyf0

hyf0 commented Mar 13, 2024

Copy link
Copy Markdown
Member

Let me take it from here. I will comment out the release ci for temp.

@hyf0 hyf0 changed the title WIP: Migrate from yarn to pnpm. Rework release workflow. feat: migrate from yarn to pnpm Mar 13, 2024

@hyf0 hyf0 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great work!

@hyf0 hyf0 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great work!

@hyf0 hyf0 merged commit fbe32d2 into main Mar 13, 2024
@hyf0 hyf0 deleted the release-flow branch March 13, 2024 13:13
@milesj

milesj commented Mar 13, 2024

Copy link
Copy Markdown
Contributor Author

Downloading and unpacking an archive over the network will still take similar times. Caching isn't really necessary.

graphite-app Bot pushed a commit that referenced this pull request May 14, 2026
…9370)

It seems this was added in #567. I'm not sure if it's still needed, but I guess no?
IWANABETHATGUY pushed a commit that referenced this pull request May 18, 2026
…9370)

It seems this was added in #567. I'm not sure if it's still needed, but I guess no?
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.

3 participants