Skip to content

Commit

Permalink
fix(codegen): dont return data from GetModuleHook in first pass
Browse files Browse the repository at this point in the history
Stencil doesn't provide any guarantees around template rendering order.
This was a concious decision made to prevent overly complex templating
usecases where templates aren't the best place to encapsulate said
logic. However, in order for templates to be able to use module hooks,
we still needed the notion of an "add to hook" and "get hook" state.
This brought the two-pass render system that we have today. The first
render pass is for adding to module hooks while the second is for
retrieving module hook data. Only the results of the second pass are
used for writing to files on the filesystem.

However, due to an oversight in the original implementation, we never
actually limited the `GetModuleHook` function to return data just during
the second pass. It returns data even during the first pass, which means
that it's possible to access data in a non-determisitic fashion during
the first pass. This commit changes that behaviour to have
`GetModuleHook` always return `[]any` during the first pass. This
ensures that there is never any order dependent module hook data being
returned.

As a reminder for reviewers, module hooks only write to the module hook
in-memory store during the first pass. They noop during the second pass.
  • Loading branch information
jaredallard committed Aug 18, 2023
1 parent f05cd8a commit 2c65ac9
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 1 deletion.
11 changes: 10 additions & 1 deletion internal/codegen/tpl_stencil.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,23 @@ type TplStencil struct {
// This is incredibly useful for allowing other modules to write
// to files that your module owns. Think of them as extension points
// for your module. The value returned by this function is always a
// []any, aka a list.
// []any, aka a list. This function only returns data during the second
// pass of stencil to ensure that template render order doesn't impact
// the data that is returned.
//
// {{- /* This returns a []any */}}
// {{ $hook := stencil.GetModuleHook "myModuleHook" }}
// {{- range $hook }}
// {{ . }}
// {{- end }}
func (s *TplStencil) GetModuleHook(name string) []any {
// On the first pass, never return any data. If we did, the data would
// be unreliably set because we don't sort the templates in any way or
// guarantee that they will be rendered in specific any order.
if s.s.isFirstPass {
return []any{}
}

k := s.s.sharedData.key(s.t.Module.Name, name)
v := s.s.sharedData.moduleHooks[k]
if v == nil {
Expand Down
6 changes: 6 additions & 0 deletions internal/codegen/tpl_stencil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,12 @@ func TestTplStencil_GetModuleHook(t *testing.T) {
return
}
}

// Ensure that GetModuleHook never returns anything other than
// `[]any` during the first pass.
if got := s.GetModuleHook(tt.args.name); !reflect.DeepEqual(got, []any{}) {
t.Errorf("TplStencil.GetModuleHook() = %v, want %v", got, []any{})
}
s.s.isFirstPass = false

// Sort the module hooks, which should be called by stencil before
Expand Down

0 comments on commit 2c65ac9

Please sign in to comment.