Skip to content

Improve pnpm setup#385

Open
kachkaev wants to merge 8 commits intodevelopfrom
pnpm
Open

Improve pnpm setup#385
kachkaev wants to merge 8 commits intodevelopfrom
pnpm

Conversation

@kachkaev
Copy link
Collaborator

@kachkaev kachkaev commented Nov 1, 2025

👋 @shd101wyy! Glad to see that you've introduced pnpm in #384! Here are some improvements – see PR comments. Happy to do the same for https://github.com/shd101wyy/vscode-markdown-preview-enhanced later if you want.

uses: actions/setup-node@v6
with:
version: latest
cache: pnpm
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Installing pnpm before node allows us to cache dependencies in CI.


- name: Install dependencies
run: pnpm install
run: pnpm install --frozen-lockfile
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ensures lockfile is in sync with package.json; otherwise fails CI (which is good because the result would be unpredicatable)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Turns out that --frozen-lockfile is enabled by default in CI. Docs: https://pnpm.io/cli/install#--frozen-lockfile

The arg is not needed.

"check:all": "npm run check:prettier && npm run check:tsc && npm run check:lint",
"check:lint": "eslint .",
"check": "pnpm check:eslint && pnpm check:prettier && pnpm check:tsc",
"check:eslint": "eslint .",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Named command after the linting tool (like in all other cases)

"build:watch": "gulp clean-out && pnpm copy:files && pnpm compile:less && concurrently \"tsc --project . --watch\" \"node build.js --watch\" \"gulp watch-less\"",
"check:all": "npm run check:prettier && npm run check:tsc && npm run check:lint",
"check:lint": "eslint .",
"check": "pnpm check:eslint && pnpm check:prettier && pnpm check:tsc",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

check is not a built-in pnpm command (unlike in yarn) so we can remove :all

package.json Outdated
"typedoc": "^0.25.1",
"typescript": "^5.2.2"
},
"packageManager": "pnpm@10.20.0",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This works similar to .tool-versions

  • All collaborators (and CI) use the same version
  • Updates are transparent
  • Calling yarn errors earlier:
This project is configured to use pnpm because /path/to/crossnote/package.json has a "packageManager" field`

@@ -0,0 +1,9 @@
ignoredBuiltDependencies:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This and onlyBuiltDependencies removes a warning during pnpm install:

 Warning ───────────────────────────────────────────────────────────────────────────────────╮
│                                                                                            │
│   Ignored build scripts: es5-ext, esbuild, fsevents, sharp.                                │
│   Run "pnpm approve-builds" to pick which dependencies should be allowed to run scripts.   │
│                                                                                            │
╰────────────────────────────────────────────────────────────────────────────────────────────╯

@kachkaev kachkaev marked this pull request as ready for review November 2, 2025 00:00
@shd101wyy shd101wyy self-requested a review November 2, 2025 06:53
Copy link
Owner

@shd101wyy shd101wyy left a comment

Choose a reason for hiding this comment

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

Thank you @kachkaev ! This PR looks good to me! Just a small question about the .tool-versions file. Should we set it to use nodej 22 instead of 20?

Happy to do the same for https://github.com/shd101wyy/vscode-markdown-preview-enhanced later if you want.

It will be great if we can also migrate https://github.com/shd101wyy/vscode-markdown-preview-enhanced ! I tried to do that yesterday but I was not sure how to configure the HaaLeo/publish-vscode-extension@v1 github action at this line. I was not sure if I should set dependencies: false there, so I reverted back to use yarn.

with:
version: latest
cache: pnpm
node-version-file: .tool-versions
Copy link
Owner

Choose a reason for hiding this comment

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

Should we update .tool-versions to 22?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can switch to Node 24 straight away because it has been promoted to LTS recently. I suggest doing it in a separate PR because Node versioning is unrelated to pnpm. WDYT?

Copy link
Owner

Choose a reason for hiding this comment

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

Yes that will be nice!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure! Feel free to merge this one if you're happy and I'll create a follow-up.

@shd101wyy shd101wyy requested a review from Copilot November 2, 2025 07:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR migrates the project from npm/yarn to pnpm as the primary package manager, improving build consistency and performance.

  • Adds pnpm workspace configuration with build dependency optimizations
  • Updates all package.json scripts to use pnpm instead of npm
  • Modernizes GitHub Actions workflows with pnpm caching and frozen lockfile installs

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
pnpm-workspace.yaml Adds pnpm workspace configuration with nodeLinker settings and build dependency controls
package.json Updates scripts from npm to pnpm, renames check/lint commands for consistency, adds packageManager field
README.md Updates installation and development instructions to reference pnpm commands
.github/workflows/typedoc.yml Reorders setup steps, upgrades setup-node to v6, adds pnpm caching and frozen-lockfile flag
.github/workflows/test.yml Reorders setup steps, upgrades setup-node to v6, adds pnpm caching, updates to use new check command
.github/workflows/release.yml Reorders setup steps, upgrades setup-node to v6, adds pnpm caching and frozen-lockfile flag

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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