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

Ensure that maybeRenderHead runs last #3821

Merged
merged 3 commits into from
Jul 6, 2022
Merged

Ensure that maybeRenderHead runs last #3821

merged 3 commits into from
Jul 6, 2022

Conversation

matthewp
Copy link
Contributor

@matthewp matthewp commented Jul 5, 2022

Changes

  • When putting a <head> into its own file, head contents (such as stylesheets) should be appended there.
  • We have a maybeRenderHead function that exists for components with no head element. This function is intended to cover the case where a page has no explicit head at all.
    • This function intends checks to see if a head has already rendered and if so, bail out.
    • Because this function was synchronous, it would run before components higher in the tree, causing the bug.
    • Maybe it be an async generator, like components are, ensures that it is put in line and doesn't run before stuff above it.
  • Fixes 🐛 BUG: Moving <head></head> into its own component produces bugs. #3819

Testing

  • Test added

Docs

N/A, bug fix

@changeset-bot
Copy link

changeset-bot bot commented Jul 5, 2022

🦋 Changeset detected

Latest commit: c5e93a0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
astro Patch
@e2e/astro-component Patch
@e2e/lit-component Patch
@e2e/preact-component Patch
@e2e/react-component Patch
@e2e/solid-component Patch
@e2e/svelte-component Patch
@e2e/e2e-tailwindcss Patch
@e2e/ts-resolution Patch

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 the pkg: astro Related to the core `astro` package (scope) label Jul 5, 2022
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.

Failing e2e test, but I like where you head is at!

@matthewp
Copy link
Contributor Author

matthewp commented Jul 5, 2022

Yeah I'm looking into the test thing.

@matthewp matthewp merged commit c2165c3 into main Jul 6, 2022
@matthewp matthewp deleted the head-in-own-component branch July 6, 2022 12:10
@astrobot-houston astrobot-houston mentioned this pull request Jul 6, 2022
SiriousHunter pushed a commit to SiriousHunter/astro that referenced this pull request Feb 3, 2023
* Ensure that maybeRenderHead runs last

* Adds a changeset

* Make work with MDX
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 BUG: Moving <head></head> into its own component produces bugs.
2 participants