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

fix: add ts-morph to dependencies #170

Closed
wants to merge 1 commit into from

Conversation

poteboy
Copy link

@poteboy poteboy commented Nov 13, 2024

During CLI execution of openapi-react-query-codegen, the library uses ts-morph as part of its internal process (as you can see here). However, ts-morph was not included as a dependency, which could lead to runtime issues when executing the CLI.

Alternatively, if adding ts-morph as a dependency feels burdensome, it may be helpful to include installation instructions for ts-morph in the documentation.

Copy link

vercel bot commented Nov 13, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
openapi-react-query-codegen ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 13, 2024 8:33am

@seriouslag
Copy link
Collaborator

It is listed as a peerDependancy. I believe all modern package managers pull the peer dependencies if not specified in the projects package.json.

If this is not the case could you provide an example.

@poteboy
Copy link
Author

poteboy commented Nov 15, 2024

@seriouslag
Actually, not all modern package managers auto-install peer dependencies. Only npm v7+, pnpm v8+ and Bun v1.x do this by default. Package managers like npm ~v6, pnpm ~v8, and Yarn (at least v1) won’t install peer dependencies automatically—you’d need to set auto-install-peers=true or use an option like install-peers.
Therefore, as mentioned in the PR description, adding installation instructions for ts-morph in the documentation could help avoid runtime issues.

Refs:

@seriouslag
Copy link
Collaborator

@seriouslag Actually, not all modern package managers auto-install peer dependencies. Only npm v7+, pnpm v8+ and Bun v1.x do this by default. Package managers like npm ~v6, pnpm ~v8, and Yarn (at least v1) won’t install peer dependencies automatically—you’d need to set auto-install-peers=true or use an option like install-peers. Therefore, as mentioned in the PR description, adding installation instructions for ts-morph in the documentation could help avoid runtime issues.

Refs:

I believe you listed a bunch of unmaintained package managers 🤷‍♂️

  • npm 6 is from 6 years ago
  • pnpm less than 8 doesn't have docs on their site.
  • yarn 1 is dead, to my knowledge

Is the only reason for this change to support these older package manager versions?

@poteboy
Copy link
Author

poteboy commented Nov 16, 2024

@seriouslag
pnpm v8 is still documented on their site, and v9 was only released 7 months ago. If that timeframe doesn't qualify as "modern," then it seems our definitions of "modern" differ. That said, the claim that "pnpm v8 doesn't have docs on their site" is simply not accurate. The emoji 🤷‍♂️ suggests this may not be a constructive discussion, so I’ll leave it at that. No hard feelings

@poteboy poteboy closed this Nov 16, 2024
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