From 9a59e24e0250617333c1a0fd89b7d52fd1c829de Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Thu, 15 Sep 2022 12:33:52 -0400 Subject: [PATCH] Ensure before-hydration is only loaded when used (#4768) * Ensure before-hydration is only loaded when used * client fix + changeset --- .changeset/fluffy-eyes-boil.md | 5 + packages/astro/src/core/app/index.ts | 12 +- packages/astro/src/core/build/generate.ts | 5 +- .../astro/src/core/build/vite-plugin-ssr.ts | 4 +- .../astro/src/runtime/server/astro-island.ts | 5 +- .../astro/src/runtime/server/hydration.ts | 5 +- packages/astro/test/before-hydration.test.js | 175 ++++++++++++++++++ .../before-hydration/astro.config.mjs | 6 + .../test/fixtures/before-hydration/deps.mjs | 1 + .../fixtures/before-hydration/package.json | 7 + .../src/components/SomeComponent.jsx | 6 + .../before-hydration/src/pages/index.astro | 11 ++ .../before-hydration/src/scripts/global.js | 1 + pnpm-lock.yaml | 8 + 14 files changed, 240 insertions(+), 11 deletions(-) create mode 100644 .changeset/fluffy-eyes-boil.md create mode 100644 packages/astro/test/before-hydration.test.js create mode 100644 packages/astro/test/fixtures/before-hydration/astro.config.mjs create mode 100644 packages/astro/test/fixtures/before-hydration/deps.mjs create mode 100644 packages/astro/test/fixtures/before-hydration/package.json create mode 100644 packages/astro/test/fixtures/before-hydration/src/components/SomeComponent.jsx create mode 100644 packages/astro/test/fixtures/before-hydration/src/pages/index.astro create mode 100644 packages/astro/test/fixtures/before-hydration/src/scripts/global.js diff --git a/.changeset/fluffy-eyes-boil.md b/.changeset/fluffy-eyes-boil.md new file mode 100644 index 000000000..e00140c6e --- /dev/null +++ b/.changeset/fluffy-eyes-boil.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +nsure before-hydration is only loaded when used diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index 1c9f7acf3..f5ce5cd8b 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -159,9 +159,15 @@ export class App { throw new Error(`Unable to resolve [${specifier}]`); } const bundlePath = manifest.entryModules[specifier]; - return bundlePath.startsWith('data:') - ? bundlePath - : prependForwardSlash(joinPaths(manifest.base, bundlePath)); + switch(true) { + case bundlePath.startsWith('data:'): + case bundlePath.length === 0: { + return bundlePath; + } + default: { + return prependForwardSlash(joinPaths(manifest.base, bundlePath)); + } + } }, route: routeData, routeCache: this.#routeCache, diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts index 378409c5a..f68659294 100644 --- a/packages/astro/src/core/build/generate.ts +++ b/packages/astro/src/core/build/generate.ts @@ -370,11 +370,8 @@ async function generatePath( if (typeof hashedFilePath !== 'string') { // If no "astro:scripts/before-hydration.js" script exists in the build, // then we can assume that no before-hydration scripts are needed. - // Return this as placeholder, which will be ignored by the browser. - // TODO: In the future, we hope to run this entire script through Vite, - // removing the need to maintain our own custom Vite-mimic resolve logic. if (specifier === BEFORE_HYDRATION_SCRIPT_ID) { - return 'data:text/javascript;charset=utf-8,//[no before-hydration script]'; + return ''; } throw new Error(`Cannot find the built path for ${specifier}`); } diff --git a/packages/astro/src/core/build/vite-plugin-ssr.ts b/packages/astro/src/core/build/vite-plugin-ssr.ts index 84352af72..3f8387f13 100644 --- a/packages/astro/src/core/build/vite-plugin-ssr.ts +++ b/packages/astro/src/core/build/vite-plugin-ssr.ts @@ -152,8 +152,8 @@ function buildManifest( // HACK! Patch this special one. if (!(BEFORE_HYDRATION_SCRIPT_ID in entryModules)) { - entryModules[BEFORE_HYDRATION_SCRIPT_ID] = - 'data:text/javascript;charset=utf-8,//[no before-hydration script]'; + // Set this to an empty string so that the runtime knows not to try and load this. + entryModules[BEFORE_HYDRATION_SCRIPT_ID] = ''; } const ssrManifest: SerializedSSRManifest = { diff --git a/packages/astro/src/runtime/server/astro-island.ts b/packages/astro/src/runtime/server/astro-island.ts index 706ccdd88..1c62b919d 100644 --- a/packages/astro/src/runtime/server/astro-island.ts +++ b/packages/astro/src/runtime/server/astro-island.ts @@ -56,7 +56,10 @@ declare const Astro: { } async childrenConnectedCallback() { window.addEventListener('astro:hydrate', this.hydrate); - await import(this.getAttribute('before-hydration-url')!); + let beforeHydrationUrl = this.getAttribute('before-hydration-url'); + if(beforeHydrationUrl) { + await import(beforeHydrationUrl); + } this.start(); } start() { diff --git a/packages/astro/src/runtime/server/hydration.ts b/packages/astro/src/runtime/server/hydration.ts index 30688237a..fdd01ce37 100644 --- a/packages/astro/src/runtime/server/hydration.ts +++ b/packages/astro/src/runtime/server/hydration.ts @@ -151,7 +151,10 @@ export async function generateHydrateScript( island.props['ssr'] = ''; island.props['client'] = hydrate; - island.props['before-hydration-url'] = await result.resolve('astro:scripts/before-hydration.js'); + let beforeHydrationUrl = await result.resolve('astro:scripts/before-hydration.js'); + if(beforeHydrationUrl.length) { + island.props['before-hydration-url'] = beforeHydrationUrl; + } island.props['opts'] = escapeHTML( JSON.stringify({ name: metadata.displayName, diff --git a/packages/astro/test/before-hydration.test.js b/packages/astro/test/before-hydration.test.js new file mode 100644 index 000000000..d8f8df3da --- /dev/null +++ b/packages/astro/test/before-hydration.test.js @@ -0,0 +1,175 @@ +import { expect } from 'chai'; +import * as cheerio from 'cheerio'; +import { loadFixture } from './test-utils.js'; +import { preact } from './fixtures/before-hydration/deps.mjs'; +import testAdapter from './test-adapter.js'; + +describe('Astro Scripts before-hydration', () => { + describe('SSG', () => { + describe('Is used by an integration', () => { + /** @type {import('./test-utils').Fixture} */ + let fixture; + + before(async () => { + fixture = await loadFixture({ + root: './fixtures/before-hydration/', + integrations: [ + preact(), + { + name: '@test/before-hydration', + hooks: { + 'astro:config:setup'({ injectScript }) { + injectScript('before-hydration', `import '/src/scripts/global.js';`); + } + } + } + ] + }); + }); + + describe('Development', () => { + /** @type {import('./test-utils').DevServer} */ + let devServer; + + before(async () => { + devServer = await fixture.startDevServer(); + }); + + after(async () => { + await devServer.stop(); + }); + + it('Is included in the astro-island', async () => { + let res = await fixture.fetch('/'); + let html = await res.text(); + let $ = cheerio.load(html); + expect($('astro-island[before-hydration-url]')).has.a.lengthOf(1); + }); + }); + + describe('Build', () => { + before(async () => { + await fixture.build(); + }); + + it('Is included in the astro-island', async () => { + let html = await fixture.readFile('/index.html') + let $ = cheerio.load(html); + expect($('astro-island[before-hydration-url]')).has.a.lengthOf(1); + }); + }); + }); + + describe('Is not used by an integration', () => { + /** @type {import('./test-utils').Fixture} */ + let fixture; + + before(async () => { + fixture = await loadFixture({ + root: './fixtures/before-hydration/' + }); + }); + + describe('Development', () => { + /** @type {import('./test-utils').DevServer} */ + let devServer; + + before(async () => { + devServer = await fixture.startDevServer(); + }); + + after(async () => { + await devServer.stop(); + }); + + it('Does include before-hydration-url on the astro-island', async () => { + let res = await fixture.fetch('/'); + let html = await res.text(); + let $ = cheerio.load(html); + expect($('astro-island[before-hydration-url]')).has.a.lengthOf(1); + }); + }); + + describe('Build', () => { + before(async () => { + await fixture.build(); + }); + + it('Does not include before-hydration-url on the astro-island', async () => { + let html = await fixture.readFile('/index.html'); + let $ = cheerio.load(html); + expect($('astro-island[before-hydration-url]')).has.a.lengthOf(0); + }); + }); + }); + }); + + describe('SSR', () => { + describe('Is used by an integration', () => { + /** @type {import('./test-utils').Fixture} */ + let fixture; + + before(async () => { + fixture = await loadFixture({ + root: './fixtures/before-hydration/', + output: 'server', + adapter: testAdapter(), + integrations: [ + preact(), + { + name: '@test/before-hydration', + hooks: { + 'astro:config:setup'({ injectScript }) { + injectScript('before-hydration', `import '/src/scripts/global.js';`); + } + } + } + ] + }); + }); + + describe('Prod', () => { + before(async () => { + await fixture.build(); + }); + + it('Is included in the astro-island', async () => { + let app = await fixture.loadTestAdapterApp(); + let request = new Request('http://example.com/'); + let response = await app.render(request); + let html = await response.text(); + let $ = cheerio.load(html); + expect($('astro-island[before-hydration-url]')).has.a.lengthOf(1); + }); + }); + }); + + describe('Is not used by an integration', () => { + /** @type {import('./test-utils').Fixture} */ + let fixture; + + before(async () => { + fixture = await loadFixture({ + root: './fixtures/before-hydration/', + output: 'server', + adapter: testAdapter(), + }); + }); + + describe('Build', () => { + before(async () => { + await fixture.build(); + }); + + it('Does not include before-hydration-url on the astro-island', async () => { + let app = await fixture.loadTestAdapterApp(); + let request = new Request('http://example.com/'); + let response = await app.render(request); + let html = await response.text(); + let $ = cheerio.load(html); + expect($('astro-island[before-hydration-url]')).has.a.lengthOf(0); + }); + }); + }); + }) +}); diff --git a/packages/astro/test/fixtures/before-hydration/astro.config.mjs b/packages/astro/test/fixtures/before-hydration/astro.config.mjs new file mode 100644 index 000000000..346e4d8b7 --- /dev/null +++ b/packages/astro/test/fixtures/before-hydration/astro.config.mjs @@ -0,0 +1,6 @@ +import { defineConfig } from 'astro/config'; +import preact from '@astrojs/preact'; + +export default defineConfig({ + integrations: [preact()] +}); diff --git a/packages/astro/test/fixtures/before-hydration/deps.mjs b/packages/astro/test/fixtures/before-hydration/deps.mjs new file mode 100644 index 000000000..aac844d57 --- /dev/null +++ b/packages/astro/test/fixtures/before-hydration/deps.mjs @@ -0,0 +1 @@ +export { default as preact } from '@astrojs/preact'; diff --git a/packages/astro/test/fixtures/before-hydration/package.json b/packages/astro/test/fixtures/before-hydration/package.json new file mode 100644 index 000000000..70e0c684d --- /dev/null +++ b/packages/astro/test/fixtures/before-hydration/package.json @@ -0,0 +1,7 @@ +{ + "name": "@test/before-hydration", + "dependencies": { + "astro": "workspace:*", + "@astrojs/preact": "workspace:*" + } +} diff --git a/packages/astro/test/fixtures/before-hydration/src/components/SomeComponent.jsx b/packages/astro/test/fixtures/before-hydration/src/components/SomeComponent.jsx new file mode 100644 index 000000000..96fd341bf --- /dev/null +++ b/packages/astro/test/fixtures/before-hydration/src/components/SomeComponent.jsx @@ -0,0 +1,6 @@ + +export default function() { + return ( +
Testing
+ ); +} diff --git a/packages/astro/test/fixtures/before-hydration/src/pages/index.astro b/packages/astro/test/fixtures/before-hydration/src/pages/index.astro new file mode 100644 index 000000000..3910745d4 --- /dev/null +++ b/packages/astro/test/fixtures/before-hydration/src/pages/index.astro @@ -0,0 +1,11 @@ +--- +import SomeComponent from '../components/SomeComponent'; +--- + + + Testing + + + + + diff --git a/packages/astro/test/fixtures/before-hydration/src/scripts/global.js b/packages/astro/test/fixtures/before-hydration/src/scripts/global.js new file mode 100644 index 000000000..d68365693 --- /dev/null +++ b/packages/astro/test/fixtures/before-hydration/src/scripts/global.js @@ -0,0 +1 @@ +console.log('testing'); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index aa5ed9760..381f2c78d 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1374,6 +1374,14 @@ importers: dependencies: astro: link:../../.. + packages/astro/test/fixtures/before-hydration: + specifiers: + '@astrojs/preact': workspace:* + astro: workspace:* + dependencies: + '@astrojs/preact': link:../../../../integrations/preact + astro: link:../../.. + packages/astro/test/fixtures/client-address: specifiers: astro: workspace:*