more resilient componentField overlap detection

HoudiniGraphql · Mar 6, 2024 · 3a6145c · 3a6145c
1 parent f50b506
commit 3a6145c
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 22 deletions.
diff --git a/packages/houdini/src/codegen/generators/definitions/schema.test.ts b/packages/houdini/src/codegen/generators/definitions/schema.test.ts
@@ -76,7 +76,7 @@ test('adds internal documents to schema', async function () {
 			directive @required on FIELD
 
 			"""@componentField marks an inline fragment as the selection for a component field"""
-			directive @componentField(field: String!, prop: String, export: String, raw: String) on FRAGMENT_DEFINITION | INLINE_FRAGMENT
+			directive @componentField(field: String!, prop: String, export: String, raw: String) on FRAGMENT_DEFINITION | INLINE_FRAGMENT | FIELD_DEFINITION
 
 			enum CachePolicy {
 			  CacheAndNetwork
@@ -165,7 +165,7 @@ test('list operations are included', async function () {
 			directive @required on FIELD
 
 			"""@componentField marks an inline fragment as the selection for a component field"""
-			directive @componentField(field: String!, prop: String, export: String, raw: String) on FRAGMENT_DEFINITION | INLINE_FRAGMENT
+			directive @componentField(field: String!, prop: String, export: String, raw: String) on FRAGMENT_DEFINITION | INLINE_FRAGMENT | FIELD_DEFINITION
 
 			enum CachePolicy {
 			  CacheAndNetwork
@@ -273,7 +273,7 @@ test('list operations are included but delete directive should not be in when we
 			directive @required on FIELD
 
 			"""@componentField marks an inline fragment as the selection for a component field"""
-			directive @componentField(field: String!, prop: String, export: String, raw: String) on FRAGMENT_DEFINITION | INLINE_FRAGMENT
+			directive @componentField(field: String!, prop: String, export: String, raw: String) on FRAGMENT_DEFINITION | INLINE_FRAGMENT | FIELD_DEFINITION
 
 			enum CachePolicy {
 			  CacheAndNetwork
@@ -394,7 +394,7 @@ test("writing twice doesn't duplicate definitions", async function () {
 			directive @required on FIELD
 
 			"""@componentField marks an inline fragment as the selection for a component field"""
-			directive @componentField(field: String!, prop: String, export: String, raw: String) on FRAGMENT_DEFINITION | INLINE_FRAGMENT
+			directive @componentField(field: String!, prop: String, export: String, raw: String) on FRAGMENT_DEFINITION | INLINE_FRAGMENT | FIELD_DEFINITION
 
 			enum CachePolicy {
 			  CacheAndNetwork

diff --git a/packages/houdini/src/codegen/transforms/schema.ts b/packages/houdini/src/codegen/transforms/schema.ts
@@ -136,7 +136,7 @@ ${
 """
 @${config.componentFieldDirective} marks an inline fragment as the selection for a component field
 """
-directive @${config.componentFieldDirective}(field: String!, prop: String, export: String, raw: String) on FRAGMENT_DEFINITION | INLINE_FRAGMENT
+directive @${config.componentFieldDirective}(field: String!, prop: String, export: String, raw: String) on FRAGMENT_DEFINITION | INLINE_FRAGMENT | FIELD_DEFINITION
 
 `
 		: ''
@@ -180,7 +180,7 @@ directive @${config.componentFieldDirective}(field: String!, prop: String, expor
 							')'
 					}
 
-					return `${fieldName}${argString}: ${config.componentScalar}!`
+					return `${fieldName}${argString}: ${config.componentScalar}!  @componentField(field: "${fieldName}")`
 				})
 				.join('\n')}
 		}

diff --git a/packages/houdini/src/codegen/validators/componentFields.test.ts b/packages/houdini/src/codegen/validators/componentFields.test.ts
@@ -33,6 +33,7 @@ const table: Row[] = [
 	{
 		title: "componentFields can't overlap with type fields",
 		pass: false,
+		nb_of_fail: 1,
 		documents: [
 			`fragment MyFragmentOne on User  @componentField(field: "firstName", prop: "user") {
 				firstName

diff --git a/packages/houdini/src/codegen/validators/componentFields.ts b/packages/houdini/src/codegen/validators/componentFields.ts
@@ -72,22 +72,23 @@ export default async function componentFields(config: Config, docs: Document[]):
 
 				// look up the type of the parent
 				const parentType = config.schema.getType(parent)
-				console.log(
-					parentType,
-					fieldValue,
-					graphql.isObjectType(parentType),
-					parentType &&
-						fieldValue &&
-						graphql.isObjectType(parentType) &&
-						parentType.getFields()[fieldValue],
-					existingField && existingField.filepath !== filepath
-				)
-				if (
-					parentType &&
-					fieldValue &&
-					((graphql.isObjectType(parentType) && parentType.getFields()[fieldValue]) ||
-						(existingField && existingField.filepath !== filepath))
-				) {
+				let conflict = false
+				if (existingField && existingField.filepath !== filepath) {
+					conflict = true
+				} else if (parentType && fieldValue) {
+					const fieldDef =
+						graphql.isObjectType(parentType) && parentType.getFields()[fieldValue]
+					if (
+						fieldDef &&
+						fieldDef.astNode?.directives?.find(
+							(dir) => dir.name.value === config.componentFieldDirective
+						)
+					) {
+						conflict = true
+					}
+				}
+
+				if (conflict) {
 					errors.push({
 						message:
 							`Duplicate component field definition for ${parent}.${fieldValue}.` +