Fix bug when with directive arguments are reused as field args

.changeset/fix-nested-fragment-argument-values.md

@@ -0,0 +1,6 @@
+---
+"houdini-core": patch
+"houdini": patch
+---
+
+Fix nil panic when generating artifacts for fragments with nested @with directives, and make artifact generation deterministic by sorting keys consistently.

packages/houdini-core/plugin/documents/artifacts/artifacts.go

@@ -105,9 +105,7 @@ func GenerateDocumentArtifacts(
 					continue
 				}
 
-				if fp != "" {
-					filepaths.Append(fp)
-				}
+				filepaths.Append(fp)
 			}
 		}()
 	}

packages/houdini-core/plugin/documents/artifacts/selection.go

@@ -10,7 +10,6 @@ import (
 	"strings"
 
 	"github.com/spf13/afero"
-
 
 	"code.houdinigraphql.com/packages/houdini-core/config"
 	"code.houdinigraphql.com/packages/houdini-core/plugin/documents/artifacts/typescript"
@@ -53,8 +52,9 @@ func writeSelectionDocument(
 	artifactPath := projectConfig.ArtifactPath(name)
 
 	// skip the write if the content hasn't changed (common on incremental runs)
-	if existing, err := afero.ReadFile(fs, artifactPath); err == nil && string(existing) == artifact {
-		return "", nil
+	if existing, err := afero.ReadFile(fs, artifactPath); err == nil &&
+		string(existing) == artifact {
+		return artifactPath, nil
 	}
 
 	// write the file to disk
@@ -1707,8 +1707,7 @@ func serializeFragmentArgument(arg *collected.ArgumentValue, level int) string {
 %s"name": {
 %s"kind": "Name",
 %s"value": "%s",
-%s},
-%s"value": "%s"`, indent1, indent2, indent2, arg.Raw, indent1, indent1, arg.Raw)
+%s}`, indent1, indent2, indent2, arg.Raw, indent1)
 	case "String", "Enum":
 		attrs = fmt.Sprintf(`
 %s"value": "%s"`, indent1, arg.Raw)

packages/houdini-core/plugin/documents/artifacts/selection_conditional_test.go

@@ -223,8 +223,7 @@ fragment UserDetails on User {
                                                 "name": {
                                                     "kind": "Name",
                                                     "value": "show",
-                                                },
-                                                "value": "show"
+                                                }
                                             }
                                         }
                                     }],
@@ -360,8 +359,7 @@ fragment UserDetails on User {
                                         "name": {
                                             "kind": "Name",
                                             "value": "show",
-                                        },
-                                        "value": "show"
+                                        }
                                     }
                                 }
                             }],

packages/houdini-core/plugin/documents/artifacts/selection_test.go

@@ -2520,8 +2520,7 @@ fragment UserPetsByAge_qkFx4 on User {
                                             "name": {
                                                 "kind": "Name",
                                                 "value": "minAge",
-                                            },
-                                            "value": "minAge"
+                                            }
                                         }
                                     }]
                                 },

packages/houdini-core/plugin/fragmentArguments/transform.go

@@ -8,8 +8,6 @@ import (
 	"sync"
 
 	"golang.org/x/sync/syncmap"
-
-
 
 	"code.houdinigraphql.com/plugins"
 	"code.houdinigraphql.com/plugins/graphql"
@@ -57,6 +55,7 @@ func Transform[PluginConfig any](ctx context.Context, db plugins.DatabasePool[Pl
     	AND selection_directives.directive = 'with'
       AND (raw_documents.current_task = $task_id OR $task_id IS NULL)
       AND (documents.processed = false OR documents.processed IS NULL)
+      AND (documents.internal = false OR documents.internal IS NULL)
     GROUP BY documents.id  
   `)
 	if err != nil {
@@ -253,7 +252,8 @@ func processDocument[PluginConfig any](
 	      parent_doc.name as document,
 	      selection_refs.id as selection_ref,
 	      selections.id as selection_id,
-	      selections.field_name as fragment,
+	      selections.field_name as current_name,
+	      COALESCE(selections.fragment_ref, selections.field_name) as fragment,
 	      fragment_doc.id as fragment_doc_id,
 	      json_group_array(
 	        json_object(
@@ -278,15 +278,14 @@ func processDocument[PluginConfig any](
 	          )
 	      END as doc_variables,
 	      fragment_doc.type_condition as type_condition,
-	      fragment_doc.raw_document as raw_document,
-	      selections.fragment_ref as fragment_ref
+	      fragment_doc.raw_document as raw_document
 	    FROM selection_directives
 	      JOIN selections ON selection_directives.selection_id = selections.id
 	      JOIN selection_refs ON selection_refs.child_id = selections.id
 	      JOIN selection_directive_arguments ON selection_directives.id = selection_directive_arguments.parent AND selection_directive_arguments.document = $document
 	      JOIN argument_values as selection_arg_values ON selection_directive_arguments."value" = selection_arg_values.id AND selection_arg_values.document = $document
 	      JOIN documents as parent_doc ON selection_refs."document" = parent_doc.id
-	      JOIN documents as fragment_doc on selections.field_name = fragment_doc.name
+	      JOIN documents as fragment_doc on COALESCE(selections.fragment_ref, selections.field_name) = fragment_doc.name
 	      LEFT JOIN document_variables on fragment_doc.id = document_variables."document"
 	      LEFT JOIN argument_values as document_variable_default_values on document_variable_default_values.id = document_variables.default_value
 	    WHERE selection_directives.directive = $with_directive
@@ -307,11 +306,11 @@ func processDocument[PluginConfig any](
 		selectionID := withSearch.GetInt64("selection_id")
 		fragmentDocID := withSearch.GetInt64("fragment_doc_id")
 		fragmentName := withSearch.GetText("fragment")
+		currentFieldName := withSearch.GetText("current_name")
 		withArgsStr := withSearch.GetText("with_args")
 		docVariablesStr := withSearch.GetText("doc_variables")
 		typeCondition := withSearch.GetText("type_condition")
 		rawDocument := withSearch.GetInt64("raw_document")
-		fragmentRef := withSearch.GetText("fragment_ref")
 
 		withArgs := []struct {
 			Name  string `json:"name"`
@@ -389,9 +388,8 @@ func processDocument[PluginConfig any](
 		hash := murmurHash(string(args))
 		newFragmentName := fragmentName + "_" + hash
 
-		// if the selection has already been transformed, don't transfor it again
-		expectedName := fragmentRef + "_" + hash
-		if fragmentName == expectedName {
+		// if the selection's field_name already matches the expected clone for this scope, skip
+		if currentFieldName == newFragmentName {
 			return
 		}
 
@@ -731,12 +729,22 @@ func cloneDocument[PluginConfig any](
 			name := statements.NoSelectionArgsDirectiveArgsSearch.GetText("name")
 			value := statements.NoSelectionArgsDirectiveArgsSearch.GetInt64("value")
 
-			// insert a directive document for the new document with the mapped value
+			// create a fresh copy of the argument value exclusively for this directive arg so
+			// it doesn't share a row with field/nested arg uses of the same variable
+			err = db.ExecStatement(statements.CopyArgumentValue, map[string]any{
+				"id":       valueMap[value],
+				"document": documentID,
+			})
+			if err != nil {
+				return
+			}
+			freshCopy := conn.LastInsertRowID()
+
 			err = db.ExecStatement(statements.InsertSelectionDirectiveArgument,
 				map[string]any{
 					"name":     name,
 					"parent":   parent,
-					"value":    valueMap[value],
+					"value":    freshCopy,
 					"document": documentID,
 				})
 		},
@@ -874,13 +882,24 @@ func cloneDocument[PluginConfig any](
 			directiveID := conn.LastInsertRowID()
 
 			for _, arg := range directive.Arguments {
-				// add the corresponding directive argument
+				// create a fresh copy exclusively for this directive arg so it doesn't
+				// share an argument_values row with field/nested arg uses of the same variable
+				err = db.ExecStatement(statements.CopyArgumentValue, map[string]any{
+					"id":       valueMap[arg.Value],
+					"document": documentID,
+				})
+				if err != nil {
+					errs.Append(plugins.WrapError(err))
+					return
+				}
+				freshCopy := conn.LastInsertRowID()
+
 				err = db.ExecStatement(
 					statements.InsertSelectionDirectiveArgument,
 					map[string]any{
 						"parent":   directiveID,
 						"name":     arg.Name,
-						"value":    valueMap[arg.Value],
+						"value":    freshCopy,
 						"document": documentID,
 					},
 				)
@@ -925,7 +944,13 @@ func cloneDocument[PluginConfig any](
 	}
 
 	// before we finish lets figure out the new scope
-	newScope, descendantVarNames, err := statements.CopyScope(ctx, db, conn, fragmentScope, documentID)
+	newScope, descendantVarNames, err := statements.CopyScope(
+		ctx,
+		db,
+		conn,
+		fragmentScope,
+		documentID,
+	)
 	if err != nil {
 		return 0, nil, nil, err
 	}
@@ -1708,12 +1733,21 @@ func (s *transformStatements[PluginConfig]) ReplaceVariables(
 ) error {
 	errs := &plugins.ErrorList{}
 
+	// track values we've already written as replacements so SQLite cursor re-scans don't
+	// pick them up and incorrectly nullify them
+	alreadyReplaced := map[int64]bool{}
+
 	db.BindStatement(search, map[string]any{"document": documentID})
 	err := db.StepStatement(ctx, search, func() {
 		parentValue := search.GetInt64("parent")
 		variableName := search.GetText("variable")
 		oldValue := search.GetInt64("value")
 
+		// skip values we already placed as replacements in this pass
+		if alreadyReplaced[oldValue] {
+			return
+		}
+
 		// if the variable name is not defined in the scope then we need to delete the original
 		// value and replace it with null otherwise we'll replace it with the scope value
 		scopeValue, ok := scope[variableName]
@@ -1748,6 +1782,9 @@ func (s *transformStatements[PluginConfig]) ReplaceVariables(
 			return
 		}
 
+		// mark the replacement so cursor re-scans don't nullify it
+		alreadyReplaced[scopeValue] = true
+
 		// by now, the value passed to the scope will replace the old value so we need to delete it
 		err = db.ExecStatement(s.DeleteValue, map[string]any{"id": oldValue})
 		if err != nil {