Nothing Special   »   [go: up one dir, main page]
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: @trpc/server@next also installing Next.js (regression in 11.0.0-rc.630) #6254

Closed
1 task done
isaac-mcfadyen opened this issue Nov 20, 2024 · 7 comments · Fixed by #6258
Closed
1 task done

bug: @trpc/server@next also installing Next.js (regression in 11.0.0-rc.630) #6254

isaac-mcfadyen opened this issue Nov 20, 2024 · 7 comments · Fixed by #6258
Labels
🐛 bug: unconfirmed

Comments

@isaac-mcfadyen
Copy link
isaac-mcfadyen commented Nov 20, 2024 •
edited
Loading

Provide environment information

  System:
    OS: macOS 15.1.1
    CPU: (10) arm64 Apple M1 Max
    Memory: 272.78 MB / 32.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.9.0 - ~/.local/state/fnm_multishells/33847_1732119510341/bin/node
    Yarn: 1.22.22 - ~/.local/state/fnm_multishells/33847_1732119510341/bin/yarn
    npm: 10.8.2 - ~/.local/state/fnm_multishells/33847_1732119510341/bin/npm
    pnpm: 9.12.1 - ~/.local/state/fnm_multishells/33847_1732119510341/bin/pnpm
    bun: 1.1.26 - ~/.bun/bin/bun
  Browsers:
    Chrome: 131.0.6778.70
    Safari: 18.1.1
  npmPackages:
    typescript: ^5.6.3 => 5.6.3

Describe the bug

Hey! 👋

Apologies if I missed an existing issue for this.

It looks like @trpc/server is now installing Next.js as a dependency.

Looks like this was a regression introducred in 11.0.0-rc.630 based on my testing. Installing 11.0.0-rc.621 locally doesn't bring in Next, but as soon as I install 11.0.0-rc.630 it starts pulling in Next.

This is significantly bloating any installs because I don't use Next.js with tRPC (especially on the server), and it's a 20MB (+40MB for SWC) install that's being pulled in with @trpc/server.

Link to reproduction

https://stackblitz.com/edit/node-6i9dpd?file=index.js

Note: just run pnpm install and then look in node_modules/.pnpm and you'll see that Next and React DOM has been installed (when only @trpc/server@11.0.0-rc.630 was installed).

To reproduce

  • Start blank Node.js project (best reproduced without any other dependencies).
  • Install @trpc/server@11.0.0-rc.621
  • Notice how install completes almost immediately and Next is not installed.
  • Install @trpc/server@11.0.0-rc.630
  • Observe as Next.js is seemingly downloaded (I can see it downloading 20MB + 40MB for SWC on my slow connection, might not be as noticeable on a faster connection).

Additional information

No response

👨‍👧‍👦 Contributing

  • 🙋‍♂️ Yes, I'd be down to file a PR fixing this bug!

Funding

  • You can sponsor this specific effort via a Polar.sh pledge below
  • We receive the pledge once the issue is completed & verified
Fund with Polar
@MattIPv4
Copy link

It looks like next is an optional dependency of the server package:

trpc/packages/server/package.json

Line 157 in 5c3057a

"next": "*",

Looking at https://docs.npmjs.com/cli/v9/configuring-npm/package-json#optionaldependencies, they're going to get installed by default unless the user explicitly opts out of installing them.

I imagine instead these should be peer dependencies, with the peer dependency meta obj marking them as optional? I think that might behave better?

@KATT
Copy link
Member
KATT commented Nov 20, 2024

Thanks. It's an easy fix, will fix tmrw unless @trpc/trpc-core beats me to it

@KATT KATT mentioned this issue Nov 22, 2024
Merged
@juliusmarminge
Copy link
Member

I imagine instead these should be peer dependencies, with the peer dependency meta obj marking them as optional? I think that might behave better?

Optional peerDependencies is a myth that I don't think any package manager honors... better to just remove them i think

@MattIPv4
Copy link

Both the pnpm and npm docs explicitly call out peerDependenciesMeta and setting a package as optional, so I would think it is honored?

@juliusmarminge
Copy link
Member

yea they're supposed to - and in the simple cases they do but i have multiple monorepos where they're complaining about peer deps that's caused by optional peer deps being pulled in where they shouldn't

@juliusmarminge
Copy link
Member
juliusmarminge commented Nov 22, 2024 •
edited
Loading

here you have an example of it breaking (it seems like it might be fixed in upcomning pnpm 10):

{
  "name": "pnpm",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "keywords": [],
  "author": "",
  "license": "ISC",
  "dependencies": {
    "ai": "^3",
    "typescript": "^5.6.3"
  },
  "packageManager": "pnpm@9.6.0"
}

where the ai SDK has all of these as optional:

CleanShot 2024-11-22 at 12 41 24@2x

yet they are installed:

CleanShot 2024-11-22 at 12 40 32@2x

@github-actions GitHub Actions
Copy link

This issue has been locked because we are very unlikely to see comments on closed issues. If you are running into a similar issue, please create a new issue. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
🐛 bug: unconfirmed
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(server): remove optionalDependencies trpc/trpc
4 participants
@KATT @isaac-mcfadyen @MattIPv4 @juliusmarminge