From 86635e035b209845b4e1cdf370a4c78451271b70 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Mon, 27 Jun 2022 10:36:41 -0400 Subject: [PATCH] Inline define:var styles rendered after the head (#3724) * Inline define:var styles rendered after the head * Adds a changeset * Remove tests that don't work with streaming --- .changeset/plenty-kids-own.md | 5 ++++ packages/astro/src/runtime/server/index.ts | 25 ++++++++++++++++++- packages/astro/test/astro-directives.test.js | 13 ++++++++-- .../src/components/Title.astro | 10 ++++++++ .../src/pages/define-vars.astro | 3 +++ .../src/components/OverwriteHeader.astro | 9 ------- .../src/components/SetAHeader.astro | 3 --- .../src/components/SetStatus.astro | 3 --- .../src/pages/child-set-status.astro | 12 --------- .../src/pages/child-tries-to-overwrite.astro | 14 ----------- .../ssr-response/src/pages/some-header.astro | 2 -- packages/astro/test/ssr-response.test.js | 19 -------------- 12 files changed, 53 insertions(+), 65 deletions(-) create mode 100644 .changeset/plenty-kids-own.md create mode 100644 packages/astro/test/fixtures/astro-directives/src/components/Title.astro delete mode 100644 packages/astro/test/fixtures/ssr-response/src/components/OverwriteHeader.astro delete mode 100644 packages/astro/test/fixtures/ssr-response/src/components/SetAHeader.astro delete mode 100644 packages/astro/test/fixtures/ssr-response/src/components/SetStatus.astro delete mode 100644 packages/astro/test/fixtures/ssr-response/src/pages/child-set-status.astro delete mode 100644 packages/astro/test/fixtures/ssr-response/src/pages/child-tries-to-overwrite.astro diff --git a/.changeset/plenty-kids-own.md b/.changeset/plenty-kids-own.md new file mode 100644 index 000000000..56ce7ca13 --- /dev/null +++ b/.changeset/plenty-kids-own.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes define:vars w/ styles used inside of components diff --git a/packages/astro/src/runtime/server/index.ts b/packages/astro/src/runtime/server/index.ts index e1ffd69e5..7f2844ed2 100644 --- a/packages/astro/src/runtime/server/index.ts +++ b/packages/astro/src/runtime/server/index.ts @@ -183,7 +183,26 @@ export async function renderComponent( } if (Component && (Component as any).isAstroComponentFactory) { - return renderToIterable(result, Component as any, _props, slots); + async function * renderAstroComponentInline(): AsyncGenerator { + let iterable = await renderToIterable(result, Component as any, _props, slots); + // If this component added any define:vars styles and the head has already been + // sent out, we need to include those inline. + if(result.styles.size && alreadyHeadRenderedResults.has(result)) { + let styles = Array.from(result.styles); + result.styles.clear(); + for(const style of styles) { + if('define:vars' in style.props) { + // We only want to render the property value and not the full stylesheet + // which is bundled in the head. + style.children = ''; + yield markHTMLString(renderElement('style', style)); + } + } + } + yield * iterable; + } + + return renderAstroComponentInline(); } if (!Component && !_props['client:only']) { @@ -408,6 +427,7 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr island.children = `${html ?? ''}${template}`; + // Scripts to prepend let prescriptType: PrescriptType = needsHydrationScript ? 'both' : needsDirectiveScript @@ -655,6 +675,7 @@ export async function renderToIterable( const Component = await componentFactory(result, props, children); if (!isAstroComponent(Component)) { + // eslint-disable-next-line no-console console.warn( `Returning a Response is only supported inside of page components. Consider refactoring this logic into something like a function that can be used in the page.` ); @@ -719,6 +740,8 @@ export async function renderHead(result: SSRResult): Promise { const styles = Array.from(result.styles) .filter(uniqueElements) .map((style) => renderElement('style', style)); + // Clear result.styles so that any new styles added will be inlined. + result.styles.clear(); const scripts = Array.from(result.scripts) .filter(uniqueElements) .map((script, i) => { diff --git a/packages/astro/test/astro-directives.test.js b/packages/astro/test/astro-directives.test.js index 161063e97..0a4bab43b 100644 --- a/packages/astro/test/astro-directives.test.js +++ b/packages/astro/test/astro-directives.test.js @@ -22,7 +22,7 @@ describe('Directives', async () => { const html = await fixture.readFile('/define-vars/index.html'); const $ = cheerio.load(html); - expect($('style')).to.have.lengthOf(1); + expect($('style')).to.have.lengthOf(2); expect($('style').toString()).to.include('--bg: white;'); expect($('style').toString()).to.include('--fg: black;'); @@ -31,9 +31,18 @@ describe('Directives', async () => { .split(' ') .find((name) => /^astro-[A-Za-z0-9-]+/.test(name)); - expect($('style').toString().replace(/\s+/g, '')).to.equal( + expect($($('style').get(0)).toString().replace(/\s+/g, '')).to.equal( `` ); + + const scopedTitleClass = $('.title') + .attr('class') + .split(' ') + .find((name) => /^astro-[A-Za-z0-9-]+/.test(name)); + + expect($($('style').get(1)).toString().replace(/\s+/g, '')).to.equal( + `` + ); }); it('set:html', async () => { diff --git a/packages/astro/test/fixtures/astro-directives/src/components/Title.astro b/packages/astro/test/fixtures/astro-directives/src/components/Title.astro new file mode 100644 index 000000000..b59303ed6 --- /dev/null +++ b/packages/astro/test/fixtures/astro-directives/src/components/Title.astro @@ -0,0 +1,10 @@ +--- + const textColor = 'red' +--- +

hello there

+ + diff --git a/packages/astro/test/fixtures/astro-directives/src/pages/define-vars.astro b/packages/astro/test/fixtures/astro-directives/src/pages/define-vars.astro index db03705ad..3df42aea1 100644 --- a/packages/astro/test/fixtures/astro-directives/src/pages/define-vars.astro +++ b/packages/astro/test/fixtures/astro-directives/src/pages/define-vars.astro @@ -1,4 +1,5 @@ --- +import Title from "../components/Title.astro" let foo = 'bar' let bg = 'white' let fg = 'black' @@ -17,5 +18,7 @@ let fg = 'black' + + </body> </html> diff --git a/packages/astro/test/fixtures/ssr-response/src/components/OverwriteHeader.astro b/packages/astro/test/fixtures/ssr-response/src/components/OverwriteHeader.astro deleted file mode 100644 index e3a99c6c3..000000000 --- a/packages/astro/test/fixtures/ssr-response/src/components/OverwriteHeader.astro +++ /dev/null @@ -1,9 +0,0 @@ ---- -let gotError = false; -try { - Astro.response.headers = new Headers(); -} catch(err) { - gotError = true; -} ---- -<div id="overwrite-error">{gotError}</div> diff --git a/packages/astro/test/fixtures/ssr-response/src/components/SetAHeader.astro b/packages/astro/test/fixtures/ssr-response/src/components/SetAHeader.astro deleted file mode 100644 index 4186561f4..000000000 --- a/packages/astro/test/fixtures/ssr-response/src/components/SetAHeader.astro +++ /dev/null @@ -1,3 +0,0 @@ ---- -Astro.response.headers.set('Seven-Eight', 'nine'); ---- diff --git a/packages/astro/test/fixtures/ssr-response/src/components/SetStatus.astro b/packages/astro/test/fixtures/ssr-response/src/components/SetStatus.astro deleted file mode 100644 index d98f3a8e1..000000000 --- a/packages/astro/test/fixtures/ssr-response/src/components/SetStatus.astro +++ /dev/null @@ -1,3 +0,0 @@ ---- -Astro.response.status = 403; ---- diff --git a/packages/astro/test/fixtures/ssr-response/src/pages/child-set-status.astro b/packages/astro/test/fixtures/ssr-response/src/pages/child-set-status.astro deleted file mode 100644 index 8b66882f1..000000000 --- a/packages/astro/test/fixtures/ssr-response/src/pages/child-set-status.astro +++ /dev/null @@ -1,12 +0,0 @@ ---- -import SetStatus from '../components/SetStatus.astro'; ---- -<html> - <head> - <title>Testing - - -

Testing

- - - diff --git a/packages/astro/test/fixtures/ssr-response/src/pages/child-tries-to-overwrite.astro b/packages/astro/test/fixtures/ssr-response/src/pages/child-tries-to-overwrite.astro deleted file mode 100644 index 35a207de7..000000000 --- a/packages/astro/test/fixtures/ssr-response/src/pages/child-tries-to-overwrite.astro +++ /dev/null @@ -1,14 +0,0 @@ ---- -import SetAHeader from '../components/SetAHeader.astro'; -import OverwriteHeader from '../components/OverwriteHeader.astro'; ---- - - - Testing - - -

Testing

- - - - diff --git a/packages/astro/test/fixtures/ssr-response/src/pages/some-header.astro b/packages/astro/test/fixtures/ssr-response/src/pages/some-header.astro index f2840ff94..ea62dfd54 100644 --- a/packages/astro/test/fixtures/ssr-response/src/pages/some-header.astro +++ b/packages/astro/test/fixtures/ssr-response/src/pages/some-header.astro @@ -1,5 +1,4 @@ --- -import SetAHeader from '../components/SetAHeader.astro'; Astro.response.headers.set('One-Two', 'three'); Astro.response.headers.set('Four-Five', 'six'); --- @@ -9,6 +8,5 @@ Astro.response.headers.set('Four-Five', 'six');

Testing

- diff --git a/packages/astro/test/ssr-response.test.js b/packages/astro/test/ssr-response.test.js index 1b517d9ee..ffabcbba4 100644 --- a/packages/astro/test/ssr-response.test.js +++ b/packages/astro/test/ssr-response.test.js @@ -32,13 +32,6 @@ describe('Using Astro.response in SSR', () => { expect(response.statusText).to.equal('Oops'); }); - it('Child component can set status', async () => { - const app = await fixture.loadTestAdapterApp(); - const request = new Request('http://example.com/child-set-status'); - const response = await app.render(request); - expect(response.status).to.equal(403); - }); - it('Can add headers', async () => { const app = await fixture.loadTestAdapterApp(); const request = new Request('http://example.com/some-header'); @@ -46,17 +39,5 @@ describe('Using Astro.response in SSR', () => { const headers = response.headers; expect(headers.get('one-two')).to.equal('three'); expect(headers.get('four-five')).to.equal('six'); - expect(headers.get('seven-eight')).to.equal('nine'); - }); - - it('Child component cannot override headers object', async () => { - const app = await fixture.loadTestAdapterApp(); - const request = new Request('http://example.com/child-tries-to-overwrite'); - const response = await app.render(request); - const headers = response.headers; - expect(headers.get('seven-eight')).to.equal('nine'); - const html = await response.text(); - const $ = cheerioLoad(html); - expect($('#overwrite-error').html()).to.equal('true'); }); });