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

Undeclared variable in fragment not used by operation #322

Open
xiongtx opened this issue May 20, 2020 · 2 comments
Open

Undeclared variable in fragment not used by operation #322

xiongtx opened this issue May 20, 2020 · 2 comments
Assignees
Labels

Comments

@xiongtx
Copy link

xiongtx commented May 20, 2020

When multiple queries are declared in a query document but the chosen operation doesn't reference a schema that requires a variable, there is a parse error.

Given the following schema, which we'll call schema:

{:objects
 {:Business {:fields {:name {:type String}
                      :owners {:type (list :Owner)}}}
  :Owner {:fields {:name {:type String}
                   :owned_businesses {:type (list :Business)
                                      :args {:location {:type String}}
                                      :resolve :placeholder}}}}

 :queries
 {:owners {:type (list :Owner)
           :resolve :placeholder}
  :businesses {:type (list :Business)
               :args {:location {:type String}}
               :resolve :placeholder}}}

And query document, which we'll call query-doc:

query BusinessesQuery($location: String) {
 businesses(location: $location) {
  owners {
   ...OwnerFields
  }
 }
}

fragment OwnerFields on Owner {
 name
 owned_businesses(location: $location)
}

query OwnerNamesQuery {
 owners {
  name
 }
}

This parses fine:

(require '[com.walmartlabs.lacinia.parser :as lacinia-parser])

(lacinia-parser/parse-query schema query-doc "BusinessesQuery")

But this throws an exception:

(lacinia-parser/parse-query schema query-doc "OwnerNamesQuery")
Argument references undeclared variable `location'.
   {:locations [{:line 10, :column 2}],
    :field :Owner/owned_businesses,
    :argument :Owner/owned_businesses.location,
    :unknown-variable :location,
    :declared-variables ()}

The problem appears to be that fragments are included without considering whether the chosen operation makes use of them.

@hlship
Copy link
Member

hlship commented Sep 25, 2020

I'm noodling on a way to do the checks for this that aren't too expensive in terms of navigating around the selection set. I'd probably want to catch recursive fragments at the same time.

@hlship hlship added the bug label Sep 25, 2020
@hlship hlship self-assigned this Sep 25, 2020
@hlship
Copy link
Member

hlship commented Dec 16, 2021

This looks like a bug, but I'm struggling to figure out a way to properly recognize defined-by-not-referenced fragments, as well as exclude fragments such as in this case. I probably need to look into spec to see what is required w.r.t validation of fragments in cases such as this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants