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

docs how to implement modules #2576

Closed

Conversation

daniele-zurico
Copy link
Contributor

TODO:

  • Update CHANGELOG.md with your change (include reference to issue & this PR)
  • Make sure all of the significant new logic is covered by tests
  • Rebase your changes on master so that they can be merged easily
  • Make sure all tests and linter rules pass

@abernix abernix added the 📝 documentation Focuses on changes to the documentation (docs) label Apr 16, 2019
@daniele-zurico
Copy link
Contributor Author

Why this PR is getting so long to be merged? There’s a specific reason?

@zero298
Copy link

zero298 commented Oct 1, 2019

Please merge this. Not seeing this in the documentation made me hesitant to actually leverage modules in my new Apollo project because I thought it might be deprecated.

@abernix abernix closed this Jun 24, 2020
@daniele-zurico
Copy link
Contributor Author

I’m sorry,
But I really don’t feel correct the behaviour. I spent time to improve the documentation I asked if there was a reason why it wasn’t merged and I never got a reply and now you close it without giving any reason?
It’s quite a disappointed behaviour to be honest

@abernix
Copy link
Member

abernix commented Jun 24, 2020

Unintentional closing! Please stand-by: #4304

@abernix abernix reopened this Jun 25, 2020
@abernix abernix changed the base branch from master to main June 25, 2020 09:04
@daniele-zurico
Copy link
Contributor Author

Given this PR is sitting here from 2y....shall I close it? I suppose you don't need it

@glasser
Copy link
Member

glasser commented Apr 14, 2021

To be honest, I have never really understood the motivation behind having "modules" as its own top-level concept that involves a dependency on our @apollographql/apollo-tools package (https://github.com/apollographql/apollo-tooling/blob/master/packages/apollo-tools/src/buildServiceDefinition.ts), when we already support passing both typeDefs and resolvers as arrays.

Is there actually a difference between

new ApolloServer({modules})

and

new ApolloServer({typeDefs: modules.map(m => m.typeDefs), resolvers: modules.map(m => m.resolvers)})

?

I'm not really clear why it's such an important feature to even warrant existing, let alone its own page in the docs. Especially given that as your PR mentions, it doesn't support custom scalars?

I also think that if we were to update these docs for today, we wouldn't want to talk about schema stitching but about federation (and perhaps some of the use cases for "modules" would just want to use federation instead?)

I'm going to close this because I don't think this is a great fit for our docs now, but I'm definitely open to users explaining to me why this option is important.

@glasser glasser closed this Apr 14, 2021
@daniele-zurico daniele-zurico deleted the docs-modules branch April 14, 2021 19:08
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
📝 documentation Focuses on changes to the documentation (docs)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants