Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Bug]: Extension install fails in monorepo due to insufficient package manager detection #4028

Closed
2 tasks done
echocrow opened this issue Jun 11, 2024 · 2 comments · Fixed by #4044
Closed
2 tasks done

[Bug]: Extension install fails in monorepo due to insufficient package manager detection #4028

echocrow opened this issue Jun 11, 2024 · 2 comments · Fixed by #4044
Labels
Type: Bug Something isn't working

Comments

@echocrow
Copy link

echocrow commented Jun 11, 2024 •
edited

Please confirm that you have:

  • Searched existing issues to see if your issue is a duplicate. (If you’ve found a duplicate issue, feel free to add additional information in a comment on it.)
  • Reproduced the issue in the latest CLI version.

In which of these areas are you experiencing a problem?

Extension

Expected behavior

Running shopify generate extension in an app folder inside a monorepo should work fine. It should scaffold the extension, and install new dependencies using the correct package manager.

Actual behavior

When creating extensions, during "Installing dependencies ..." the CLI will error out with:

── external error ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

Error coming from `npm install react@^18.0.0 --save-prod`

Command failed with exit code 1: npm install react@^18.0.0 --save-prod
npm WARN ignoring workspace config at /path/to/repo/apps/my-shopify-app/.npmrc
npm ERR! code EUNSUPPORTEDPROTOCOL
npm ERR! Unsupported URL Type "workspace:": workspace:^0.0.0

The CLI did not correct the right package manager (see "Reproduction steps" below). Expected pnpm, but npm was used as a fallback.

Looks like the internal getPackageManager(fromDirectory) tries to check for local lockfiles, but in a monorepo, that would only be found at the root repo directory, but not in the nested app/package directory.

Possible solutions(?):

  • Add a flag to skip installation
    • already present in app build and app dev via --skip-dependencies-installation
  • Improve package manager detection (TBD how)
  • Add flag to force a specific package manager
    • not preferred, but already present in app init via --package-manager

Verbose output

...
[DATETIME]: Reading the content of file at package.json...
[DATETIME]: Obtaining the dependency manager in directory /path/to/repo/apps/my-shopify-app...
[DATETIME]: Reading the content of file at package.json...
...

Reproduction steps

Sample monorepo project setup:

./
├── package.json
├── pnpm-lock.yaml
└─┬ apps/
  └─┬ my-shopify-app/
    ├── package.json
    └─┬ extensions/
      └── ...

Sample step:

  • Run cd apps/my-shopify-app
  • Run shopify generate extension
  • Pick any extension that will auto-trigger a package install step, e.g. Checkout UI, and follow the remaining prompts

Operating System

Multi platform

Shopify CLI version (check your project's package.json if you're not sure)

v3.61.2

Shell

No response

Node version (run node -v if you're not sure)

v20.11.1

What language and version are you using in your application?

No response
@echocrow echocrow added the Type: Bug Something isn't working label Jun 11, 2024
@amcaplan amcaplan mentioned this issue Jun 13, 2024
Merged
6 tasks
@amcaplan
Copy link
Contributor

Hey @echocrow, thanks for the bug report! I've merged a PR to update the package manager detection logic, which should cover your use case.

I should note, however, that the CLI itself uses workspaces in a setup where the workspace root is the app project root, and each extension is a workspace. So if the apps themselves are workspaces, you are now creating nested workspaces.

It's a bit difficult to find explicit documentation about this, but my understanding is that support for nested workspaces among package managers is still a bit iffy, and we can't guarantee that the CLI will work correctly in this setup. There may be a number of bugs we're unable to solve, and since this seems to be a rare setup among our users so far, it is also questionable whether we'll be able to dedicate time even if bugs are fundamentally solvable.

If you're willing to take on those risks, then I earnestly wish you the best of luck pioneering the use of nested workspaces for Shopify apps. Let us know how it goes!

@echocrow
Copy link
Author

echocrow commented Jun 17, 2024 •
edited

@amcaplan Thanks for the update and fast PR! TIL, npm prefix also works in a pnpm monorepo. ✅

RE workspaces: Cheers for the precautionary note, and well understood! We've already been using this setup successfully for a few months now, including another non-Shopify app that also uses nested ./extensions packages. Both pnpm and turborepo have been working perfectly as expected. Our pnpm-workspace.yaml file looks something like this:

packages:
  - 'apps/*'
  - 'apps/another-app/extensions/*'
  - 'apps/shopify/extensions/*'
  - 'packages/*'

Thanks to your PR, we won't have to work around the Shopify CLI anymore. (Previously we'd be passing --skip-dependencies-installation wherever possible, and be limited to scaffolding new extensions in a throw-away repo). There may indeed still be some small caveats, e.g. when invoking the CLI from the monorepo root instead of after cd apps/shopify. We'll be sure to keep an eye out for those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Type: Bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Search for lockfile in project root to identify package manager in workspaces Shopify/cli
2 participants
@echocrow @amcaplan