pingdotgg / uploadthing

File uploads for modern web devs
https://uploadthing.com
MIT License
3.95k stars 285 forks source link

Packages sometimes fail to install on NPM #14

Closed t3dotgg closed 1 year ago

t3dotgg commented 1 year ago

People reported this during stream, haven't had a chance to repro

ViableSystemModel commented 1 year ago

Reproduced.

Windows 10 Pro 21H1 uploadthing@3.0.1 npm@8.15.0 and npm@9.5.1

Got the same error for both versions of npm:

npm ERR! code E404 npm ERR! 404 Not Found - GET https://registry.npmjs.org/@uploadthing%2feslint-config - Not found npm ERR! 404 npm ERR! 404 '@uploadthing/eslint-config@0.1.0' is not in this registry. npm ERR! 404 npm ERR! 404 Note that you can also install from a npm ERR! 404 tarball, folder, http url, or git url.

I changed line 22 in ./package.json from:

"@uploadthing/eslint-config": "0.1.0", to: "@uploadthing/eslint-config": "file:./common/eslint-config-custom",

That allowed the package to install. But then running npm run build gave the following error:

build turbo run build

• Packages in scope: @example/appdir, @example/pagedir, @uploadthing/eslint-config, @uploadthing/react, @uploadthing/tsconfig, docs, uploadthing • Running build in 7 packages • Remote caching disabled uploadthing:build: cache miss, executing c55d788e4ca361d5 docs:build: cache miss, executing bf964c276951d810 uploadthing:build: ERROR: command finished with error: exec: "pnpm": executable file not found in %PATH% exec: "pnpm": executable file not found in %PATH%

Tasks: 0 successful, 2 total Cached: 0 cached, 2 total Time: 1.766s

ERROR run failed: command exited (1)

Seems like turbo is seeing pnpm-lock.yaml and pnpm-workspace.yaml and using pnpm as its command, which is a sensible guess. I don't know enough about turborepo to say for sure, but it seems like making this able to build with multiple package managers is going to involve multiple lock files that could fall out of sync with each other. Might be better to just specify that pnpm is mandatory in the README.

ViableSystemModel commented 1 year ago

There's a potentially relevant comment here: https://github.com/vercel/turbo/issues/1630#issuecomment-1299753909:

In this scenario you have a pnpm-lock.yaml and a package-lock.json in the root.

No matter what we attempt to detect it will make somebody unhappy. There is no reasonable way for us to ensure that we guess correctly in this situation.

We provide a method to skip that inference by specifying packageManager and no matter what we decide to do, that will be a requirement in this scenario.

I think this means that the only way to support multiple package managers in a turborepo project is:

  1. Remove "packageManager": "pnpm@7.27.0" from your package.json
  2. Add package-lock.json, yarn.lock, and pnpm-lock.yaml to your .gitignore
  3. Proceed to do the actual testing and debugging to verify your commands all work with each package manager

So.... sounds like sticking with pnpm and closing this issue is the way to go.

juliusmarminge commented 1 year ago

Seems like turbo is seeing pnpm-lock.yaml and pnpm-workspace.yaml and using pnpm as its command

Yes, we use pnpm as our package manager here. You shouldnt use npm or yarn when developing in this repo. I thought the packageManager field restricted this? Of maybe npm doesnt respect that 🙄

We dont have a contributing guide but we probably should add some quick steps on how to get going.

Closing this.