-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Make mocking configuration less confusing. #1835
Conversation
Setting mocks to false will always disable mocking.
@deftomat: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/ |
@@ -254,7 +254,7 @@ export class ApolloServerBase { | |||
}); | |||
} | |||
|
|||
if (mocks || typeof mockEntireSchema !== 'undefined') { | |||
if (mocks || (typeof mockEntireSchema !== 'undefined' && mocks !== false)) { |
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.
Hey @deftomat Thanks for looking into this! I've been meaning to look at it. Question: wouldn't this end up doing the same thing?
if (mocks || (typeof mockEntireSchema !== 'undefined' && mocks !== false)) { | |
if (mocks) { |
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.
To my understanding, this is the current intended behavior:
mocks: false
- no matter what, no mocks get added
mocks: true
- by default, mocks all resolvers
mockEntireSchema: false
to overwrite this
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.
Well, in my code, you can enable mocks by setting mockEntireSchema
without bothering about mocks
property.
By looking at the next lines, you can see that it is an intended behaviour to be able to enable mocks just by using mockEntireSchema
.
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.
Yep! That makes more sense actually 👌 Thanks for clarifying @deftomat
@deftomat Can you add this line to
|
@JakeDawkins done! |
Thanks for the help @deftomat! |
We had a few WTF? situations 😉 with the following Apollo server config:
Everyone in our team assumed that you can disable the mocks by setting the
mocks
property to false.However, according to:
apollo-server/packages/apollo-server-core/src/ApolloServer.ts
Line 257 in a12673a
The mocks will be enabled when you provide
mocksEntireSchema
together withmocks
. It doesn't matter ifmocks
istrue
orfalse
or an object.I'm not sure if this is an intended behaviour but I can definitely say that it is really confusing.
Can we update it to disable mocks when
mocks
property isfalse
?(Feel free to close this PR if the current behaviour is an intended behaviour)