Skip to content

Rename varsMap to argsMap#23

Merged
jneurock merged 2 commits intokloeckner-i:masterfrom
jgwhite:args-map
Jul 10, 2019
Merged

Rename varsMap to argsMap#23
jneurock merged 2 commits intokloeckner-i:masterfrom
jgwhite:args-map

Conversation

@jgwhite
Copy link
Copy Markdown
Contributor

@jgwhite jgwhite commented Jul 9, 2019

Previously this option worked with variable names rather than argument names. We believe mapping argument names is more useful and more likely to be the expected behavior. This patch removes any notion of mapping variable names and focuses strictly on argument names whilst also renaming the option to make it clearer what is being mapped.

Previously the `vars` that were being passed down from the query mock
were in fact the args for the root query selection and therefore already
massaged by the GraphQL executor to have the correct names. This means
that variables to be used further down the graph could either be missing
or have the wrong names.

This is tricky to explain succintly, please see the test case for a
concrete example. In the test case, we are dealing with a scenario where
the variable is used in both the root of the query *and* further down
with a different name. If it were used further down and *not* in the
root query then the variable would simply be absent prior to this patch.

Co-authored-by: Chad Carbert <chadcarbert@me.com>
Comment thread addon/mocks/query.js Outdated
Previously this option worked with variable names rather than argument
names. We believe mapping argument names is more useful and more likely
to be the expected behavior. This patch removes any notion of mapping
variable names and focuses strictly on argument names whilst also
renaming the option to make it clearer what is being mapped.

Closes kloeckner-i#22

Co-authored-by: Chad Carbert <chadcarbert@me.com>
@jneurock jneurock merged commit 30e6b8a into kloeckner-i:master Jul 10, 2019
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