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 a bundle of dependency issues #6393

Merged
merged 1 commit into from May 5, 2022
Merged

Fix a bundle of dependency issues #6393

merged 1 commit into from May 5, 2022

Conversation

glasser
Copy link
Member

@glasser glasser commented May 5, 2022

Some packages were depending on other packages that were only declared
as transitive dependencies. Clean this up by adding appropriate
dependencies (or in one case, just re-declaring ValueOrPromise and
dropping the apollo-server-types dependency).

The peer dep one is a bit funny. But "Y has a peer dep on Z" means
"when you install Y you need to install Z", and so if X depends on Y,
then when you install X you need to install Z... so sure, that means X
needs to have a peer dep on Z too, I guess.

Fixes #6389.
Fixes #6390.
Fixes #6391.
Fixes #6392.

@glasser glasser requested a review from trevor-scheer May 5, 2022 22:03
@netlify
Copy link

netlify bot commented May 5, 2022

Deploy Preview for apollo-server-docs canceled.

Name Link
🔨 Latest commit e3c078e
🔍 Latest deploy log https://app.netlify.com/sites/apollo-server-docs/deploys/627449bf0178010008eca25f

Some packages were depending on other packages that were only declared
as transitive dependencies. Clean this up by adding appropriate
dependencies (or in one case, just re-declaring ValueOrPromise and
dropping the apollo-server-types dependency).

The peer dep one is a bit funny.  But "Y has a peer dep on Z" means
"when you install Y you need to install Z", and so if X depends on Y,
then when you install X you need to install Z... so sure, that means X
needs to have a peer dep on Z too, I guess.

Fixes #6389.
Fixes #6390.
Fixes #6391.
Fixes #6392.
@codesandbox-ci
Copy link

codesandbox-ci bot commented May 5, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit e3c078e:

Sandbox Source
Apollo Server Typescript Configuration
Apollo Server Configuration

@glasser glasser merged commit 2706fc2 into main May 5, 2022
@glasser glasser deleted the glasser/fix-deps branch May 5, 2022 22:08
@@ -22,6 +22,7 @@
},
"homepage": "https://github.com/apollographql/apollo-server#readme",
"dependencies": {
"@types/express": "4.17.13",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it need to be pinned? ):

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's pinned like this in apollo-server-express so I figured doing the same thing was reasonable.

We've had a lot of challenges with the Express DefinitelyTyped packages before. They factored out a huge part of the types from @types/express into @types/express-serve-static-core but the dependency of the former on the latter doesn't declare a specific version, so it's easy to accidentally install a pair of versions that don't work together. So unfortunately trying to pin more precisely does seem helpful. If they get behind we're happy to update everything...

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants