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.

Sign up for GitHub

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

Jump to bottom

Prevent eager rendering of head content in multi-level MDX layout #6107

Merged
merged 4 commits into from
Feb 2, 2023
Merged

Prevent eager rendering of head content in multi-level MDX layout #6107

merged 4 commits into from
Feb 2, 2023

Conversation

matthewp
Copy link
Contributor

@matthewp matthewp commented Feb 2, 2023 •
edited
Loading

Changes

  • MDX components rendering to strings and not to a stream. That means the technique introduced in Simplified head injection #6034 which relies on messaging order doesn't prevent head from being injected into the body when using a typical MDX + slot setup.
  • This new change uses the concept of a "scope", this way we know if we are rendering inside of a JSX component (MDX) that contains a layout. If we are inside of both of these things we know that we don't need to emit the head because it will be done so by the layout itself.
  • Fixes MDX files with components insert script in layout slot #6057

Testing

  • Had an existing test which didn't correct test that the head content was inside of the head tag. This fixes that.

Docs

N/A, bug fix

@matthewp matthewp requested a review from a team as a code owner February 2, 2023 17:44
@changeset-bot
Copy link

changeset-bot bot commented Feb 2, 2023 •
edited
Loading

🦋 Changeset detected

Latest commit: c202b3a

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 github-actions bot added pkg: astro Related to the core `astro` package (scope) pkg: integration Related to any renderer integration (scope) labels Feb 2, 2023
@matthewp matthewp mentioned this pull request Feb 2, 2023
Closed
1 task
bholmesdev
bholmesdev reviewed Feb 2, 2023
View reviewed changes
Copy link
Contributor

@bholmesdev bholmesdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some nits, but eagereat job!

packages/astro/src/@types/astro.ts Outdated Show resolved Hide resolved
packages/astro/src/runtime/server/jsx.ts Outdated Show resolved Hide resolved
packages/astro/src/runtime/server/render/astro/factory.ts Show resolved Hide resolved
@matthewp matthewp merged commit 9bec6bc into main Feb 2, 2023
@matthewp matthewp deleted the mdx-head-placement branch February 2, 2023 19:21
@astrobot-houston astrobot-houston mentioned this pull request Feb 2, 2023
Merged
matthewp added a commit that referenced this pull request Feb 3, 2023
@matthewp
Prevent eager rendering of head content in multi-level MDX layout (#6107
c8d601e 
)

* Prevent eager rendering of head content in multi-level MDX layout

* Adding a changeset

* Remove old comment

* Keep track of slot position as well
@matthewp matthewp mentioned this pull request Feb 6, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bholmesdev bholmesdev bholmesdev left review comments

Assignees
No one assigned
Labels
pkg: astro Related to the core `astro` package (scope) pkg: integration Related to any renderer integration (scope)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

MDX files with components insert script in layout slot
2 participants
@matthewp @bholmesdev