Skip to content

Conversation

theimbender
Copy link
Contributor

@theimbender theimbender commented Aug 5, 2025

This PR adds support for the --pnpm CLI option, and pnpm support in general, for the create-contentful-app tool. This PR does not convert the entire monorepo to use pnpm.

Why is this needed?

For third party apps using pnpm as a package manager, trying to create Contentful apps with create-contentful-app via a custom npm script (or any kind of script, really) can end up erroring out when the new app's dependencies are installed because of how different local versions of pnpm and npm are handled.

The commands given in console for installing dependencies, and for creating app definitions, are misleading in this case. There was also some ambiguity and unexpected behavior in the handling of when both --yarn and --npm flags were passed, and npm would be preferred even if it wasn't the current package manager. With this change, multiple package manager flags are ignored and the currently active package manager will be used; this is the same behavior as omitting a package manager flag at all.

This PR updates all areas that package managers are referenced, including the Segment.io analytics track call. Comprehensive test coverage is provided.

@theimbender theimbender marked this pull request as ready for review August 5, 2025 16:32
@theimbender theimbender requested review from a team as code owners August 5, 2025 16:32
@theimbender theimbender force-pushed the theimbender/support-pnpm branch from e175ea3 to 4a3afc0 Compare August 21, 2025 13:34
Copy link

@Copilot 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 adds support for pnpm as a package manager option to the create-contentful-app tool, while improving the overall package manager handling logic. The change allows users to specify --pnpm as a CLI option and automatically detects pnpm when it's the active package manager.

  • Adds pnpm detection and support throughout the codebase
  • Improves package manager conflict resolution by defaulting to the active package manager when multiple flags are provided
  • Updates documentation and success messages to include pnpm commands

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/types.ts Adds pnpm option to CLIOptions and defines PackageManager type
src/utils.ts Implements pnpm detection, package manager normalization, and conflict resolution logic
src/index.ts Updates CLI option parsing, package installation, and success message generation for pnpm
src/template.ts Adds pnpm-lock.json cleanup to template preparation
src/analytics.ts Updates analytics tracking to support pnpm package manager
test/utils.spec.ts Comprehensive test coverage for new package manager detection and normalization functions
README.md Updates documentation to reflect pnpm support and new behavior

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

michaelpineirocontentful added a commit that referenced this pull request Aug 22, 2025
Based on [this PR](#2631) , with minor tweaks.
michaelpineirocontentful added a commit that referenced this pull request Aug 22, 2025
Based on [this PR](#2631) , with minor tweaks.
michaelpineirocontentful added a commit that referenced this pull request Aug 22, 2025
Based on [this PR](#2631) , with minor tweaks.
michaelpineirocontentful added a commit that referenced this pull request Aug 22, 2025
Based on [this PR](#2631) , with minor tweaks.
michaelpineirocontentful added a commit that referenced this pull request Aug 22, 2025
* feat(pnpm): support pnpm as package manager

* feat: support-pnpm-package-manager [ZEND-6808]

Based on [this PR](#2631) , with minor tweaks.

---------

Co-authored-by: Timothy Heimbender <[email protected]>
Copy link
Contributor

@michaelpineirocontentful michaelpineirocontentful left a comment

Choose a reason for hiding this comment

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

I made a few small tweaks, mostly to make detectActivePackageManager() more comprehensive. I have opened a PR against your branch. If you could review it and confirm it looks good, we can merge it in and then get this PR merged to main.

To ensure everything works as expected, I also published an alpha release that includes both of our commits. I tested it, and it worked successfully.

@theimbender
Copy link
Contributor Author

I made a few small tweaks, mostly to make detectActivePackageManager() more comprehensive. I have opened a PR against your branch. If you could review it and confirm it looks good, we can merge it in and then get this PR merged to main.

To ensure everything works as expected, I also published an alpha release that includes both of our commits. I tested it, and it worked successfully.

Cool, I've tested on my local as well and it works as expected. I merged and just had one follow-up commit where I converted those requires to imports in the utils spec. Thank you for jumping on this!

@michaelpineirocontentful michaelpineirocontentful merged commit e2785c1 into contentful:main Aug 26, 2025
19 checks passed
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.

2 participants