Jetpack CLI: Mount host pnpm store in Docker for better performance #46487
+113
−8
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Auto-detect and mount the host's pnpm store into the monorepo Docker container, allowing it to reuse already-downloaded packages. This significantly speeds up pnpm install operations and reduces disk usage. Implementation: - Detect host pnpm store using 'pnpm store path' command - Strip version suffix to get parent directory for version management - Fallback to default location (~/.local/share/pnpm/store) if pnpm not installed - Only mount if store directory exists (graceful fallback) - Configure pnpm via .npmrc in container's /root directory - Refactored docker run to use bash arrays for cleaner code The feature is: - Always-on by default (no opt-in required) - Zero configuration - Fully backward compatible - Transparent to users
When the pnpm store location changes, set CI=true for that container run to let pnpm auto-approve node_modules cleanup. This prevents the ERR_PNPM_ABORTED_REMOVE_MODULES_DIR_NO_TTY error without manually removing directories. The script now: - Detects when store-dir configuration changes in .npmrc - Sets CI=true only when store location changed (not always) - Shows warning message to inform user of automatic rebuild - Lets pnpm handle the cleanup properly
Detect store mismatch by checking node_modules/.modules.yaml and run
a separate migration step before the user's command to preserve
interactivity.
When migration is needed:
1. Run 'pnpm install' with CI=true to auto-approve node_modules cleanup
2. Run the user's original command with CI="${CI:-}" for full interactivity
This ensures:
- Automatic migration for users adopting the host store feature
- No ERR_PNPM_ABORTED_REMOVE_MODULES_DIR_NO_TTY errors
- Full interactive prompt support in jetpack commands
- No overhead after initial migration (single docker run)
The detection checks node_modules/.modules.yaml storeDir field to
determine if migration is needed, which is more reliable than checking
.npmrc changes.
kraftbj
added
the
[Type] Enhancement
Contributor
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes:
Other information:
Jetpack product discussion
N/A - CLI tooling improvement
Does this pull request change what data or activity we track or use?
No
Testing instructions:
jp init-hooksto set up git hooksjp --helpto verify the CLI works correctly