-
Notifications
You must be signed in to change notification settings - Fork 3.2k
fix: updated npmrc with hoist patterns #8271
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
Conversation
WalkthroughConfiguration update to Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
.npmrc(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: sriramveeraghanta
Repo: makeplane/plane PR: 7672
File: pnpm-workspace.yaml:8-9
Timestamp: 2025-08-29T08:45:15.953Z
Learning: The makeplane/plane repository uses pnpm v10.12.1, which supports onlyBuiltDependencies configuration in pnpm-workspace.yaml files.
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Assume `--moduleResolution bundler` resolution strategy for modern web projects (Vite, Next.js, etc.)
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Use `import type { Type } from "mod" with { "resolution-mode": "import" }` for specific module resolution contexts (TypeScript 5.3+)
🔇 Additional comments (1)
.npmrc (1)
48-54: Verify hoist patterns resolve correctly and don't cause version conflicts.After fixing the pattern syntax, verify that:
- All five patterns (
typescript,eslint,@plane/*,vite,turbo) correctly resolve and hoist to the root node_modules.- No version conflicts arise from hoisting these packages across different workspaces (especially critical for
@plane/*internal packages that may have stricter version requirements).- The hoisting doesn't break any workspace-specific dependency constraints.
Run a
pnpm installon your local environment and verify the hoisting behavior, or usepnpm list -rto inspect the dependency tree and confirm packages are hoisted correctly.
| public-hoist-pattern[] = typescript | ||
| public-hoist-pattern[] = eslint | ||
| public-hoist-pattern[] = *@plane/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct the glob pattern for @plane scoped packages.
The pattern *@plane/* uses incorrect glob syntax and won't match @plane/* scoped packages as intended. In glob patterns, the leading * matches any characters before @plane/, so it would match foo@plane/bar instead of @plane/something.
Apply this diff to fix the pattern:
-public-hoist-pattern[] = *@plane/*
+public-hoist-pattern[] = @plane/*🤖 Prompt for AI Agents
In .npmrc around lines 50 to 52, the glob pattern for @plane scoped packages is
incorrect; replace the existing public-hoist-pattern[] = *@plane/* entry with
public-hoist-pattern[] = @plane/* so the pattern matches packages scoped under
@plane (e.g., @plane/foo) instead of incorrectly matching strings that contain
@plane in the middle.
There was a problem hiding this 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 attempts to add public-hoist-pattern configurations to .npmrc to improve dependency resolution performance by hoisting commonly used tools (typescript, eslint, vite, turbo, and @plane/* packages) to the root node_modules. However, there are critical compatibility issues with the existing configuration and unintended lockfile changes.
- Adds hoisting patterns for frequently used development tools
- Introduces a configuration conflict with the existing
node-linker = isolatedsetting - Inadvertently downgrades
caniuse-litedependency from a newer to an older version
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
.npmrc |
Adds public-hoist-pattern configurations for typescript, eslint, vite, turbo, and @plane/* packages; conflicts with existing node-linker = isolated setting |
pnpm-lock.yaml |
Adds [email protected] resolution and snapshot entries; downgrades Next.js's caniuse-lite dependency from version 1.0.30001759 to 1.0.30001756 |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.