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

Improve head injection behavior #2436

Merged
merged 5 commits into from
Mar 2, 2022
Merged

Improve head injection behavior #2436

merged 5 commits into from
Mar 2, 2022

Conversation

natemoo-re
Copy link
Member

@natemoo-re natemoo-re commented Jan 20, 2022

Changes

Testing

Existing tests updated

Docs

N/A

@changeset-bot
Copy link

changeset-bot bot commented Jan 20, 2022

⚠️ No Changeset found

Latest commit: b499ffd

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Jan 20, 2022
@FredKSchott
Copy link
Member

Closing as stale, feel free to reopen when ready!

@natemoo-re natemoo-re reopened this Feb 25, 2022
@github-actions github-actions bot added the test label Feb 25, 2022
@natemoo-re natemoo-re changed the base branch from main to next February 25, 2022 23:08
@natemoo-re natemoo-re changed the title [WIP] add renderHead util to server Add renderHead util to server Feb 25, 2022
@github-actions github-actions bot removed the test label Feb 25, 2022
@github-actions github-actions bot added pkg: example Related to an example package (scope) feat: markdown Related to Markdown (scope) test labels Feb 25, 2022
@natemoo-re natemoo-re marked this pull request as ready for review February 25, 2022 23:59
@github-actions github-actions bot added 🚨 action Modifies GitHub Actions and removed pkg: example Related to an example package (scope) feat: markdown Related to Markdown (scope) test labels Feb 26, 2022
@github-actions github-actions bot added pkg: example Related to an example package (scope) test labels Feb 28, 2022
@github-actions github-actions bot removed pkg: example Related to an example package (scope) test labels Feb 28, 2022
@github-actions github-actions bot added the test label Mar 1, 2022
@natemoo-re natemoo-re changed the title Add renderHead util to server Improve head injection behavior Mar 1, 2022
@github-actions github-actions bot removed 🚨 action Modifies GitHub Actions test labels Mar 2, 2022
Comment on lines -438 to +445
export async function renderPage(result: SSRResult, Component: AstroComponentFactory, props: any, children: any) {
const template = await renderToString(result, Component, props, children);
export async function renderHead(result: SSRResult) {
Copy link
Member Author

Choose a reason for hiding this comment

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

No more renderPage entrypoint, just use renderToString.

Comment on lines +429 to +431
if (template.indexOf('<!--astro:head-->') > -1) {
template = template.replace('<!--astro:head-->', await renderHead(result));
}
Copy link
Member Author

Choose a reason for hiding this comment

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

<!--astro:head--> is injected by the compiler. If we see this, it's at the end of the <head> element and we should inject the links/styles/scripts.

return links.join('\n') + styles.join('\n') + scripts.join('\n') + template; // if no </head>, prepend styles & scripts
}
return template.substring(0, headPos) + links.join('\n') + styles.join('\n') + scripts.join('\n') + template.substring(headPos);
return unescapeHTML(links.join('\n') + styles.join('\n') + scripts.join('\n') + '\n' + '<!--astro:head:injected-->');
Copy link
Member Author

Choose a reason for hiding this comment

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

We also add a <!--astro:head:injected--> comment to flag that we've already injected the links/styles/scripts.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I understand after reading the code below.

@@ -47,7 +44,6 @@ async function compile(config: AstroConfig, filename: string, source: string, vi
// use `sourcemap: "both"` so that sourcemap is included in the code
// result passed to esbuild, but also available in the catch handler.
const transformResult = await transform(source, {
as: isPage ? 'document' : 'fragment',
Copy link
Member Author

Choose a reason for hiding this comment

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

No more as: 'document'|'fragment'! We no longer care, every Astro file is parsed the same.

Comment on lines +102 to +105
// handle final head injection if it hasn't happened already
if (html.indexOf("<!--astro:head:injected-->") == -1) {
html = await renderHead(result) + html;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Slight deviation from RFC0007... if we haven't injected any head elements yet, the user may not have rendered a <head> element so we just prepend them to the file.

html = await renderHead(result) + html;
}
// cleanup internal state flags
html = html.replace("<!--astro:head:injected-->", '');
Copy link
Member Author

Choose a reason for hiding this comment

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

Remove our flag for a nicer output.

@matthewp
Copy link
Contributor

matthewp commented Mar 2, 2022

lgtm

@natemoo-re natemoo-re merged commit b0cedc8 into next Mar 2, 2022
@natemoo-re natemoo-re deleted the feat/render-head branch March 2, 2022 22:00
natemoo-re added a commit that referenced this pull request Mar 3, 2022
* feat: add renderHead util to server

* feat: remove `layouts` from config, Vite plugin

* fix: improve head injection during rendering

* chore: update compiler

* fix: do not escape links
natemoo-re added a commit that referenced this pull request Mar 4, 2022
* feat: add renderHead util to server

* feat: remove `layouts` from config, Vite plugin

* fix: improve head injection during rendering

* chore: update compiler

* fix: do not escape links
natemoo-re added a commit that referenced this pull request Mar 4, 2022
* feat: add renderHead util to server

* feat: remove `layouts` from config, Vite plugin

* fix: improve head injection during rendering

* chore: update compiler

* fix: do not escape links
natemoo-re added a commit that referenced this pull request Mar 4, 2022
* feat: add renderHead util to server

* feat: remove `layouts` from config, Vite plugin

* fix: improve head injection during rendering

* chore: update compiler

* fix: do not escape links
natemoo-re added a commit that referenced this pull request Mar 4, 2022
* feat: add renderHead util to server

* feat: remove `layouts` from config, Vite plugin

* fix: improve head injection during rendering

* chore: update compiler

* fix: do not escape links
natemoo-re added a commit that referenced this pull request Mar 8, 2022
* feat: add renderHead util to server

* feat: remove `layouts` from config, Vite plugin

* fix: improve head injection during rendering

* chore: update compiler

* fix: do not escape links
natemoo-re added a commit that referenced this pull request Mar 9, 2022
* feat: add renderHead util to server

* feat: remove `layouts` from config, Vite plugin

* fix: improve head injection during rendering

* chore: update compiler

* fix: do not escape links
natemoo-re added a commit that referenced this pull request Mar 9, 2022
* feat: add renderHead util to server

* feat: remove `layouts` from config, Vite plugin

* fix: improve head injection during rendering

* chore: update compiler

* fix: do not escape links
natemoo-re added a commit that referenced this pull request Mar 9, 2022
* Unflag the static build (#2652)

* Unflag the static build

* Only set legacyBuild to false if experimentalSSR is true

* Use legacy build when we have to

* Put a few more tests into legacy mode

* Last two

* Make astro-basic use the legacy build

* Adds a changeset

* Mark the lit test as legacy

* Update yarn lock

* Update based on feedback

* Add --legacy-build flag

* Move astro-basic test to use static build (#2682)

* Move some tests over to the static build (#2677)

* Move some tests over to the static build

* Fix assets tests

* Fix the assets tests

* Fix for the client:only components

* Moves asset tests to the static build

* Move postcss test over to static build

* Bring back legacy build for astro-basic test

* Move astro-basic test to use static build

* Migrate more tests to the static build (#2693)

* fix: disable HMR during build (#2684)

* Migrate more tests to the static build

* Only prepend links in non-legacy mode

* Add the 0-css tests

* Convert all CSS tests to the static build

* Migrate Astro global tests

* Remove .only

* Fix static build tests

* Migrate a few more

* More tests

* Move the lit test back to legacy

* Increase the test timeout

Co-authored-by: Nate Moore <[email protected]>

* Improve `head` injection behavior (#2436)

* feat: add renderHead util to server

* feat: remove `layouts` from config, Vite plugin

* fix: improve head injection during rendering

* chore: update compiler

* fix: do not escape links

* chore: enter `pre` mode

* Replace `send` with `sirv` (#2713)

* remove send

* Create thick-ravens-chew.md

* I feel like I'm going to screw something up

* working finally!

* rewrite req.url

* Add tiny bit of doc

* Update .gitignore

Co-authored-by: Evan Boehs <[email protected]>

* Move remaining tests to the static build (#2712)

* Move lit test to the static build

* Migrate astro-env plugin to work in the static build

* Do not remove vite:define

* Adds a changeset

* Add a warning when passing the --experimental-static-build flag (#2718)

* Add a warning when passing the --experimental-static-build flag

* Disable the lint warning

* [ci] release (next) (#2721)

* [ci] release (next)

* chore: update changeset

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Nate Moore <[email protected]>

* 404 page (#2719)

* Fix: build to 404.html in the static build

* Adds a changeset

* fix pnpm install missing peer deps

* fix svelte version in workspace

* fix lockfile

* fix(webapi): add dev script

* improve preview reliability (#2739)

* improve preview reliability - fix broken tests

* shamefully hoist to unblock

* remove lit from test running

* chore: update lockfile

Co-authored-by: Matthew Phillips <[email protected]>
Co-authored-by: Evan Boehs <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Fred K. Schott <[email protected]>
SiriousHunter pushed a commit to SiriousHunter/astro that referenced this pull request Feb 3, 2023
* Unflag the static build (withastro#2652)

* Unflag the static build

* Only set legacyBuild to false if experimentalSSR is true

* Use legacy build when we have to

* Put a few more tests into legacy mode

* Last two

* Make astro-basic use the legacy build

* Adds a changeset

* Mark the lit test as legacy

* Update yarn lock

* Update based on feedback

* Add --legacy-build flag

* Move astro-basic test to use static build (withastro#2682)

* Move some tests over to the static build (withastro#2677)

* Move some tests over to the static build

* Fix assets tests

* Fix the assets tests

* Fix for the client:only components

* Moves asset tests to the static build

* Move postcss test over to static build

* Bring back legacy build for astro-basic test

* Move astro-basic test to use static build

* Migrate more tests to the static build (withastro#2693)

* fix: disable HMR during build (withastro#2684)

* Migrate more tests to the static build

* Only prepend links in non-legacy mode

* Add the 0-css tests

* Convert all CSS tests to the static build

* Migrate Astro global tests

* Remove .only

* Fix static build tests

* Migrate a few more

* More tests

* Move the lit test back to legacy

* Increase the test timeout

Co-authored-by: Nate Moore <[email protected]>

* Improve `head` injection behavior (withastro#2436)

* feat: add renderHead util to server

* feat: remove `layouts` from config, Vite plugin

* fix: improve head injection during rendering

* chore: update compiler

* fix: do not escape links

* chore: enter `pre` mode

* Replace `send` with `sirv` (withastro#2713)

* remove send

* Create thick-ravens-chew.md

* I feel like I'm going to screw something up

* working finally!

* rewrite req.url

* Add tiny bit of doc

* Update .gitignore

Co-authored-by: Evan Boehs <[email protected]>

* Move remaining tests to the static build (withastro#2712)

* Move lit test to the static build

* Migrate astro-env plugin to work in the static build

* Do not remove vite:define

* Adds a changeset

* Add a warning when passing the --experimental-static-build flag (withastro#2718)

* Add a warning when passing the --experimental-static-build flag

* Disable the lint warning

* [ci] release (next) (withastro#2721)

* [ci] release (next)

* chore: update changeset

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Nate Moore <[email protected]>

* 404 page (withastro#2719)

* Fix: build to 404.html in the static build

* Adds a changeset

* fix pnpm install missing peer deps

* fix svelte version in workspace

* fix lockfile

* fix(webapi): add dev script

* improve preview reliability (withastro#2739)

* improve preview reliability - fix broken tests

* shamefully hoist to unblock

* remove lit from test running

* chore: update lockfile

Co-authored-by: Matthew Phillips <[email protected]>
Co-authored-by: Evan Boehs <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Fred K. Schott <[email protected]>
SiriousHunter pushed a commit to SiriousHunter/astro that referenced this pull request Feb 3, 2023
* Unflag the static build (withastro#2652)

* Unflag the static build

* Only set legacyBuild to false if experimentalSSR is true

* Use legacy build when we have to

* Put a few more tests into legacy mode

* Last two

* Make astro-basic use the legacy build

* Adds a changeset

* Mark the lit test as legacy

* Update yarn lock

* Update based on feedback

* Add --legacy-build flag

* Move astro-basic test to use static build (withastro#2682)

* Move some tests over to the static build (withastro#2677)

* Move some tests over to the static build

* Fix assets tests

* Fix the assets tests

* Fix for the client:only components

* Moves asset tests to the static build

* Move postcss test over to static build

* Bring back legacy build for astro-basic test

* Move astro-basic test to use static build

* Migrate more tests to the static build (withastro#2693)

* fix: disable HMR during build (withastro#2684)

* Migrate more tests to the static build

* Only prepend links in non-legacy mode

* Add the 0-css tests

* Convert all CSS tests to the static build

* Migrate Astro global tests

* Remove .only

* Fix static build tests

* Migrate a few more

* More tests

* Move the lit test back to legacy

* Increase the test timeout

Co-authored-by: Nate Moore <[email protected]>

* Improve `head` injection behavior (withastro#2436)

* feat: add renderHead util to server

* feat: remove `layouts` from config, Vite plugin

* fix: improve head injection during rendering

* chore: update compiler

* fix: do not escape links

* chore: enter `pre` mode

* Replace `send` with `sirv` (withastro#2713)

* remove send

* Create thick-ravens-chew.md

* I feel like I'm going to screw something up

* working finally!

* rewrite req.url

* Add tiny bit of doc

* Update .gitignore

Co-authored-by: Evan Boehs <[email protected]>

* Move remaining tests to the static build (withastro#2712)

* Move lit test to the static build

* Migrate astro-env plugin to work in the static build

* Do not remove vite:define

* Adds a changeset

* Add a warning when passing the --experimental-static-build flag (withastro#2718)

* Add a warning when passing the --experimental-static-build flag

* Disable the lint warning

* [ci] release (next) (withastro#2721)

* [ci] release (next)

* chore: update changeset

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Nate Moore <[email protected]>

* 404 page (withastro#2719)

* Fix: build to 404.html in the static build

* Adds a changeset

* fix pnpm install missing peer deps

* fix svelte version in workspace

* fix lockfile

* fix(webapi): add dev script

* improve preview reliability (withastro#2739)

* improve preview reliability - fix broken tests

* shamefully hoist to unblock

* remove lit from test running

* chore: update lockfile

Co-authored-by: Matthew Phillips <[email protected]>
Co-authored-by: Evan Boehs <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Fred K. Schott <[email protected]>
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.

3 participants