From 5021ae80e1fe8ba680d019913fd341cb21605e25 Mon Sep 17 00:00:00 2001
From: bumblepie <luke@bumblepie.space>
Date: Sun, 17 May 2020 19:43:37 +1200
Subject: [PATCH] Improve lookahead visibility for aliased fields (#662)
* Improve lookahead visibility for aliased fields
- Add a method to access the children of look ahead structs
- Make the behaviour around accessing aliased lookahead fields more consistent
* Deprecate old Lookahead methods for accessing child selections
---
juniper/CHANGELOG.md | 4 ++
juniper/src/executor/look_ahead.rs | 103 +++++++++++++++++++++--------
2 files changed, 81 insertions(+), 26 deletions(-)
diff --git a/juniper/CHANGELOG.md b/juniper/CHANGELOG.md
index 8d6f2d09..35e6dabf 100644
--- a/juniper/CHANGELOG.md
+++ b/juniper/CHANGELOG.md
@@ -32,6 +32,8 @@ See [#618](https://github.com/graphql-rust/juniper/pull/618).
- Better error messages for all proc macros (see
[#631](https://github.com/graphql-rust/juniper/pull/631)
+- Improved lookahead visibility for aliased fields (see [#662](https://github.com/graphql-rust/juniper/pull/631))
+
## Breaking Changes
- `juniper::graphiql` has moved to `juniper::http::graphiql`
@@ -71,6 +73,8 @@ See [#618](https://github.com/graphql-rust/juniper/pull/618).
Rename `http::tests::HTTPIntegration` as `http::tests::HttpIntegration`
and add support for `application/graphql` POST request.
+- When using LookAheadMethods to access child selections, children are always found using their alias if it exists rather than their name (see [#662](https://github.com/graphql-rust/juniper/pull/631)). These methods are also deprecated in favour of the new `children` method.
+
# [[0.14.2] 2019-12-16](https://github.com/graphql-rust/juniper/releases/tag/juniper-0.14.2)
- Fix incorrect validation with non-executed operations [#455](https://github.com/graphql-rust/juniper/issues/455)
diff --git a/juniper/src/executor/look_ahead.rs b/juniper/src/executor/look_ahead.rs
index c9dbba6b..c5e05b6a 100644
--- a/juniper/src/executor/look_ahead.rs
+++ b/juniper/src/executor/look_ahead.rs
@@ -332,14 +332,19 @@ pub struct ConcreteLookAheadSelection<'a, S: 'a> {
/// A set of common methods for `ConcreteLookAheadSelection` and `LookAheadSelection`
pub trait LookAheadMethods<S> {
- /// Get the name of the field represented by the current selection
+ /// Get the (potentially aliased) name of the field represented by the current selection
fn field_name(&self) -> &str;
/// Get the the child selection for a given field
+ /// If a child has an alias, it will only match if the alias matches `name`
+ #[deprecated(note = "please use `children` to access the child selections instead")]
fn select_child(&self, name: &str) -> Option<&Self>;
- /// Check if a given field exists
+ /// Check if a given child selection with a name exists
+ /// If a child has an alias, it will only match if the alias matches `name`
+ #[deprecated(note = "please use `children` to access the child selections instead")]
fn has_child(&self, name: &str) -> bool {
+ #[allow(deprecated)]
self.select_child(name).is_some()
}
@@ -357,8 +362,12 @@ pub trait LookAheadMethods<S> {
self.arguments().iter().find(|a| a.name == name)
}
- /// Get the top level children for the current selection
+ /// Get the (possibly aliased) names of the top level children for the current selection
+ #[deprecated(note = "please use `children` to access the child selections instead")]
fn child_names(&self) -> Vec<&str>;
+
+ /// Get an iterator over the children for the current selection
+ fn children(&self) -> Vec<&Self>;
}
impl<'a, S> LookAheadMethods<S> for ConcreteLookAheadSelection<'a, S> {
@@ -367,7 +376,7 @@ impl<'a, S> LookAheadMethods<S> for ConcreteLookAheadSelection<'a, S> {
}
fn select_child(&self, name: &str) -> Option<&Self> {
- self.children.iter().find(|c| c.name == name)
+ self.children.iter().find(|c| c.field_name() == name)
}
fn arguments(&self) -> &[LookAheadArgument<S>] {
@@ -375,10 +384,7 @@ impl<'a, S> LookAheadMethods<S> for ConcreteLookAheadSelection<'a, S> {
}
fn child_names(&self) -> Vec<&str> {
- self.children
- .iter()
- .map(|c| c.alias.unwrap_or(c.name))
- .collect()
+ self.children.iter().map(|c| c.field_name()).collect()
}
fn has_arguments(&self) -> bool {
@@ -388,6 +394,10 @@ impl<'a, S> LookAheadMethods<S> for ConcreteLookAheadSelection<'a, S> {
fn has_children(&self) -> bool {
!self.children.is_empty()
}
+
+ fn children(&self) -> Vec<&Self> {
+ self.children.iter().collect()
+ }
}
impl<'a, S> LookAheadMethods<S> for LookAheadSelection<'a, S> {
@@ -398,7 +408,7 @@ impl<'a, S> LookAheadMethods<S> for LookAheadSelection<'a, S> {
fn select_child(&self, name: &str) -> Option<&Self> {
self.children
.iter()
- .find(|c| c.inner.name == name)
+ .find(|c| c.inner.field_name() == name)
.map(|s| &s.inner)
}
@@ -407,10 +417,7 @@ impl<'a, S> LookAheadMethods<S> for LookAheadSelection<'a, S> {
}
fn child_names(&self) -> Vec<&str> {
- self.children
- .iter()
- .map(|c| c.inner.alias.unwrap_or(c.inner.name))
- .collect()
+ self.children.iter().map(|c| c.inner.field_name()).collect()
}
fn has_arguments(&self) -> bool {
@@ -420,6 +427,13 @@ impl<'a, S> LookAheadMethods<S> for LookAheadSelection<'a, S> {
fn has_children(&self) -> bool {
!self.children.is_empty()
}
+
+ fn children(&self) -> Vec<&Self> {
+ self.children
+ .iter()
+ .map(|child_selection| &child_selection.inner)
+ .collect()
+ }
}
#[cfg(test)]
@@ -1290,6 +1304,7 @@ query Hero {
}
#[test]
+ #[allow(deprecated)]
fn check_select_child() {
let lookahead: LookAheadSelection<DefaultScalarValue> = LookAheadSelection {
name: "hero",
@@ -1441,12 +1456,14 @@ fragment heroFriendNames on Hero {
}
#[test]
+ #[allow(deprecated)]
fn check_visitability() {
let docs = parse_document_source::<DefaultScalarValue>(
"
query Hero {
hero(episode: EMPIRE) {
name
+ aliasedName: name
friends {
name
}
@@ -1474,22 +1491,56 @@ query Hero {
assert_eq!(args[0].value(), &LookAheadValue::Enum("EMPIRE"));
assert!(look_ahead.has_children());
- assert_eq!(look_ahead.child_names(), vec!["name", "friends"]);
+ assert_eq!(
+ look_ahead.child_names(),
+ vec!["name", "aliasedName", "friends"]
+ );
+ let mut children = look_ahead.children().into_iter();
- let child0 = look_ahead.select_child("name").unwrap();
- assert_eq!(child0.field_name(), "name");
- assert!(!child0.has_arguments());
- assert!(!child0.has_children());
+ let name_child = children.next().unwrap();
+ assert!(look_ahead.has_child("name"));
+ assert_eq!(name_child, look_ahead.select_child("name").unwrap());
+ assert_eq!(name_child.name, "name");
+ assert_eq!(name_child.alias, None);
+ assert_eq!(name_child.field_name(), "name");
+ assert!(!name_child.has_arguments());
+ assert!(!name_child.has_children());
- let child1 = look_ahead.select_child("friends").unwrap();
- assert_eq!(child1.field_name(), "friends");
- assert!(!child1.has_arguments());
- assert!(child1.has_children());
- assert_eq!(child1.child_names(), vec!["name"]);
+ let aliased_name_child = children.next().unwrap();
+ assert!(look_ahead.has_child("aliasedName"));
+ assert_eq!(
+ aliased_name_child,
+ look_ahead.select_child("aliasedName").unwrap()
+ );
+ assert_eq!(aliased_name_child.name, "name");
+ assert_eq!(aliased_name_child.alias, Some("aliasedName"));
+ assert_eq!(aliased_name_child.field_name(), "aliasedName");
+ assert!(!aliased_name_child.has_arguments());
+ assert!(!aliased_name_child.has_children());
- let child2 = child1.select_child("name").unwrap();
- assert!(!child2.has_arguments());
- assert!(!child2.has_children());
+ let friends_child = children.next().unwrap();
+ assert!(look_ahead.has_child("friends"));
+ assert_eq!(friends_child, look_ahead.select_child("friends").unwrap());
+ assert_eq!(friends_child.name, "friends");
+ assert_eq!(friends_child.alias, None);
+ assert_eq!(friends_child.field_name(), "friends");
+ assert!(!friends_child.has_arguments());
+ assert!(friends_child.has_children());
+ assert_eq!(friends_child.child_names(), vec!["name"]);
+
+ assert!(children.next().is_none());
+
+ let mut friends_children = friends_child.children().into_iter();
+ let child = friends_children.next().unwrap();
+ assert!(friends_child.has_child("name"));
+ assert_eq!(child, friends_child.select_child("name").unwrap());
+ assert_eq!(child.name, "name");
+ assert_eq!(child.alias, None);
+ assert_eq!(child.field_name(), "name");
+ assert!(!child.has_arguments());
+ assert!(!child.has_children());
+
+ assert!(friends_children.next().is_none());
} else {
panic!("No Operation found");
}