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

fix(codegen): dont return data from GetModuleHook in first pass, shuffle templates #272

Closed

Conversation

jaredallard
Copy link
Contributor

@jaredallard jaredallard commented Aug 18, 2023

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.

Also as part of this, to prevent further accidental leakage of file
ordering, we now sort templates randomly on each stencil run. This
mirrors mentality behind the Go maps to help prevent developers from
relying on a certain key ordering.

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.

Updates `devbase` and `stencil-golang` to move to Go 1.20.7
@jaredallard jaredallard requested a review from a team as a code owner August 18, 2023 03:23
@jaredallard jaredallard force-pushed the jaredallard/fix/dont-return-data-get-module-hook branch 2 times, most recently from 5d39227 to 2330940 Compare August 18, 2023 03:44
@jaredallard jaredallard changed the title fix(codegen): dont return data from GetModuleHook in first pass fix(codegen): dont return data from GetModuleHook in first pass, shuffle templates Aug 18, 2023
@jaredallard jaredallard force-pushed the jaredallard/fix/dont-return-data-get-module-hook branch from 2330940 to 8f5bcd5 Compare August 18, 2023 03:44
…fle templates

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.

Also as part of this, to prevent further accidental leakage of file
ordering, we now sort templates randomly on each stencil run. This
mirrors mentality behind the Go maps to help prevent developers from
relying on a certain key ordering.

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.
@jaredallard jaredallard force-pushed the jaredallard/fix/dont-return-data-get-module-hook branch from 8f5bcd5 to e515c3b Compare August 18, 2023 03:45
Base automatically changed from jaredallard/feat/go-1.20 to main August 21, 2023 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant