fix(runtime): check propagation hint for head in tree #13776
Conversation
🦋 Changeset detectedLatest commit: 35fcf1b The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
the
pkg: astro
CodSpeed Performance ReportMerging #13776 will not alter performanceComparing Summary
|
| return hint === 'in-tree' || hint === 'self'; | ||
| } | ||
|
|
||
| export function getPropagationHint( |
There was a problem hiding this comment.
don't we have this same function somewhere else? Or is it different? If the same we should reuse.
There was a problem hiding this comment.
The other function is in the same file, few lines up. It's different and it checks if hint === 'in-tree' || hint === 'self'.
There was a problem hiding this comment.
Shouldn't that function use this one then?
| const propagationHint = getPropagationHint(result, componentFactory); | ||
| result._metadata.headInTree = | ||
| result.componentMetadata.get(componentFactory.moduleId!)?.containsHead ?? false; | ||
| result.componentMetadata.get(componentFactory.moduleId!)?.containsHead ?? propagationHint === 'in-tree'; |
There was a problem hiding this comment.
should this also be checking for propagationHint === 'self'?
There was a problem hiding this comment.
It seems not. When working with propagation, the Astro engine assigns in-tree hint when a server island is used inside a component.
There was a problem hiding this comment.
Actually, thinking about it again, the headInTree variable is intended to be used to know if the <head> element is anywhere in the tree of components. That is the actual <head> tag. It's not intended to be used to know if we should do head propagation.
The purpose is to support cases where there is no explicit <head> element used in the page (a legacy mistake, imo).
There was a problem hiding this comment.
Uhm OK. I will create a different flag then, Thank you for your comments!
2 participants
Changes
While working with CSP and server islands, I found out that we don't use the
PropagationHintto determine if the head is the tree of the components or not.As explained here:
astro/packages/astro/src/types/public/internal.ts
Lines 251 to 261 in 90293de
The
in-treehint should signal our rendering engine if there's a component that we must wait, so it can render the head. The wasn't done in the code base. When I added the check, I could see the server island was correctly awaited, and eventually the head was rendered.I preferred to send the fix here because it's unrelated to the work I've been doing.
Testing
I tested it manually. We don't have special components that would trigger situation for now, so the CI should stay green.
Docs
N/A