-
Notifications
You must be signed in to change notification settings - Fork 1.5k
adds the transitive @types/express-serve-static-core dependency as a direct devDependency #1078
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
adds the transitive @types/express-serve-static-core dependency as a direct devDependency #1078
Conversation
d6140d7 to
133fceb
Compare
133fceb to
c00f0b9
Compare
commit: |
|
Hello, Could you please provide the error that you have from pnpm, as well as your pnpm config? Going forward, if |
c00f0b9 to
fa497aa
Compare
Hey @KKonstantinov -- here's the current build error when running tl;dr; here is that pnpm and other strict package managers don't allow imports to transitive dependencies, so we solve that here by adding hoisting the |
fa497aa to
1207d30
Compare
1207d30 to
83cfb46
Compare
|
I'm not sure we should be adding additional dependencies to satisfy a package manager we dont use in this repo. We should do this only if there is a strict mode with npm we can enable which requires this. |
@mattzcarey Fair point about Counterpoint is that we do have a true dependency on I'd love to help close out some open issues in this repo. I picked up #572 because @felixweinberger marked as We can either a) merge this PR to resolve the issue, or b) respond to the original issue to let the author know we're not going to address it. |
mattzcarey
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this!
This PR adds the transitive
@types/express-serve-static-coredependency as a direct devDependency to fix builds via strict package managers likepnpm. This is necessary because types fromexpress-serve-static-coreare currently referenced directly in https://github.com/modelcontextprotocol/typescript-sdk/blob/main/src/server/auth/middleware/bearerAuth.ts#L23-L30Motivation and Context
Fixes #572
How Has This Been Tested?
pnpm install+pnpm buildon a fresh clone of this branch now succeedsrm -rf node_modules/ && pnpm i && pnpm buildalso succeedsrm -rf node_modules/ && npm i && npm run buildalso succeeds.Breaking Changes
n/a
Types of changes
Checklist
Additional context