Fix head propagation for MDX components #7838
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 14d8b8f 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
bluwy
commented
Jul 27, 2023
| @@ -37,6 +37,7 @@ export class AstroComponentInstance { | |||
| } | |||
|
|
|||
| async init(result: SSRResult) { | |||
| if (this.returnValue !== undefined) return this.returnValue; | |||
There was a problem hiding this comment.
This part makes sure this.returnValue is cached. Otherwise we're calling render() then init(), which causes double head rendering.
bluwy
commented
Jul 27, 2023
Comment on lines
152
to
167
| // Recursively calls component instances that might have head content | ||
| // to be propagated up. | ||
| async function bufferHeadContent(result: SSRResult) { | ||
| const iterator = result._metadata.propagators.values(); | ||
| while (true) { | ||
| const { value, done } = iterator.next(); | ||
| if (done) { | ||
| break; | ||
| } | ||
| // Call component instances that might have head content to be propagated up. | ||
| const returnValue = await value.init(result); | ||
| if (isHeadAndContent(returnValue)) { | ||
| result._metadata.extraHead.push(returnValue.head); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This code is what we had before I did the refactor, so no changes made here.
ematipico
approved these changes
Jul 28, 2023
|
Yup, the conflicts should go away after I revert #7843 |
|
Nevermind, I can't seem to revert that PR now. Will manually do it in this PR, then merge. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
Revert this part #7782 (comment)
Fixes MDX fail (Presumably also fixes markdoc but it doesn't seem to have tests cover head propagation)
I'm actually not sure how MDX components can get their head propagated, because it involves attaching themselves to
result._metadata.propagators(which I didn't see any for MDX specifically). But reverting this part for now fix it.Testing
Ran MDX and markdoc test locally. Others tests should also pass.
Docs
n/a. bug fix.