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.

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

v3 does not work with other package managers #156

Closed
zanechua opened this issue Aug 14, 2023 · 14 comments · Fixed by #166 or #174
Closed

v3 does not work with other package managers #156

zanechua opened this issue Aug 14, 2023 · 14 comments · Fixed by #166 or #174
Assignees

Comments

@zanechua
Copy link

Currently getting the following error thrown npm ERR! Cannot read properties of null (reading 'matches') in https://github.com/zanechua/comment-worker/actions/runs/5857501953/job/15879483256

Seems to be an incompatibility since I am using pnpm instead of npm?

Can we have support for making use of other package managers to install wrangler other than npm ?

@zanechua
Copy link
Author

Had to downgrade one of my dependencies as it was a peer dependency issue. Error only occurs with npm and not pnpm though.

zanechua/comment-worker@691d844

@dotansimha
Copy link

dotansimha commented Aug 16, 2023

Same here, since upgraded to v3 of the action, the deploy action fails with:

📥 Installing Wrangler
  Running command: npm install wrangler@3.4.0
  npm ERR! Cannot read properties of null (reading 'matches')

We are using pnpm in that repo, I suspect that pnpm install wrangler@v is not playing nice with that?

@JacobMGEvans
Copy link
Contributor

Hmm, I can look into making it package agnostic and fallback to npm or pnpm (honestly I prefer pnpm myself too lol)

@beary
Copy link

beary commented Aug 28, 2023

I deployed successfully without wrangler-action, this is my config. You can install wrangler on your deploy step or add it into you package.json.

      - name: Deploy
        env:
          CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN}}
          CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID}}
        run: |
          pnpm install --save-dev wrangler
          pnpm exec wrangler pages deploy public --project-name=<Your project name>

@AdiRishi
Copy link
Contributor

Just ran into this myself. With the current state of wrangler-action you basically can't use pnpm in CI. Got the same exception

📥 Installing Wrangler
  Running command: npm install wrangler@latest
  npm ERR! Cannot read properties of null (reading 'matches')
  
  npm ERR! A complete log of this run can be found in: /home/runner/.npm/_logs/2023-08-29T05_44_20_839Z-debug-0.log
  Error: 🚨 Action failed

It's worth noting that it works fine with yarn v3. Discovered this bug as I moved to pnpm.

@AdiRishi
Copy link
Contributor

@JacobMGEvans this may not be resolved.

Just tied to test the fix by using uses: cloudflare/wrangler-action@main and got the following output

Run cloudflare/wrangler-action@main
  with:
    accountId: ***
    apiToken: ***
    wranglerVersion: latest
    preCommands: echo "*** pre commands ***"
wrangler r2 bucket create turborepo-cache || true
  echo "******"
  
    command: deploy
    quiet: false
  env:
    PNPM_HOME: /home/runner/setup-pnpm/node_modules/.bin
Error: File not found: '/home/runner/work/_actions/cloudflare/wrangler-action/main/dist/index.mjs'

Looks like it's able to detect pnpm but there is some bug with executing index.mjs? Here is the link to the failing action in case that helps.

@JacobMGEvans
Copy link
Contributor

I'm pretty sure I haven't released this fix yet.

@AdiRishi
Copy link
Contributor

AdiRishi commented Sep 21, 2023

I'm pretty sure I haven't released this fix yet.

Hmm my apologies then. I assumed #166 was the complete fix. I was able to use by using the @main tag on wrangler-action.

@nix6839
Copy link
Contributor

nix6839 commented Sep 21, 2023

I was able to use by using the @main tag on wrangler-action.

main branch doesn't have /dist/index.mjs. It's a output of a build.

@AdiRishi
Copy link
Contributor

I was able to use by using the @main tag on wrangler-action.

main branch doesn't have /dist/index.mjs. It's a output of a build.

Right, that makes sense! My bad 😅

@hubertott
Copy link

Just an FYI I just got this on my recent deployment and noticed this work was just merged into release v3.2.0

file:///home/runner/work/_actions/cloudflare/wrangler-action/v3/dist/index.mjs:2923
throw new Error("Package manager is not detected");
^

Error: Package manager is not detected
at realPackageManager (file:///home/runner/work/_actions/cloudflare/wrangler-action/v3/dist/index.mjs:2923:11)
at file:///home/runner/work/_actions/cloudflare/wrangler-action/v3/dist/index.mjs:2925:48
at file:///home/runner/work/_actions/cloudflare/wrangler-action/v3/dist/index.mjs:3117:3
at ModuleJob.run (node:internal/modules/esm/module_job:192:25)
at async DefaultModuleLoader.import (node:internal/modules/esm/loader:228:24)
at async loadESM (node:internal/process/esm_loader:40:7)
at async handleMainPromise (node:internal/modules/run_main:66:12)

Node.js v20.5.0

@nix6839
Copy link
Contributor

nix6839 commented Oct 2, 2023

Just an FYI I just got this on my recent deployment and noticed this work was just merged into release v3.2.0

@hubertott Please try the following

- uses: cloudflare/wrangler-action@v3
  with:
    packageManager: "npm" # change npm to yarn or pnpm

@hubertott
Copy link

hubertott commented Oct 3, 2023

@nix6839 thanks for that.

Maybe I am blind, but there doesn't appear to be any reference to this in the README.
Should this not be considered a breaking change?

In our case we use a mono-repo structure where the yarn install process happens at the root level. We are also specifying the working directory of the worker files. All in all our folder structure doesn't play well with the auto-detect.

Feels like the error message could be a little more informative. Like you suggestion of setting the packageManager.

@nix6839
Copy link
Contributor

nix6839 commented Oct 3, 2023

Sorry, I didn't document when I contributed.
I'm not a native english speaker, so it would be great if someone who speaks English better than me could contribute.

A short description of the feature:

  • Read the packageManager settings. If it's one of npm, yarn, or pnpm, use it.
  • If the packageManager is not set or is invalid, find package-lock.json or yarn.lock or pnpm-lock.yaml in workingDirectory, and use the package manager for it.
  • If all of the above steps fail, throw error.

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 a pull request may close this issue.

7 participants