Skip to content

Commit

Permalink
fix: handle external hoisted scripts correctly (#9437)
Browse files Browse the repository at this point in the history
  • Loading branch information
@dkobierski
dkobierski authored Dec 19, 2023
1 parent c32e607 commit 354a62c
Show file tree
Hide file tree
Showing 3 changed files with 184 additions and 21 deletions.
5 changes: 5 additions & 0 deletions .changeset/warm-bats-eat.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fixes incorrect hoisted script paths when custom rollup output file names are configured
3 changes: 2 additions & 1 deletion packages/astro/src/core/build/plugins/plugin-manifest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,9 @@ function buildManifest(
if (route.prerender || !pageData) continue;
const scripts: SerializedRouteInfo['scripts'] = [];
if (pageData.hoistedScript) {
const shouldPrefixAssetPath = pageData.hoistedScript.type === 'external';
const hoistedValue = pageData.hoistedScript.value;
const value = hoistedValue.endsWith('.js') ? prefixAssetPath(hoistedValue) : hoistedValue;
const value = shouldPrefixAssetPath ? prefixAssetPath(hoistedValue) : hoistedValue;
scripts.unshift(
Object.assign({}, pageData.hoistedScript, {
value,
Expand Down
197 changes: 177 additions & 20 deletions packages/astro/test/ssr-hoisted-script.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,46 +11,203 @@ async function fetchHTML(fixture, path) {
return html;
}

describe('Hoisted scripts in SSR', () => {
/** @type {import('./test-utils').AstroInlineConfig} */
const defaultFixtureOptions = {
root: './fixtures/ssr-hoisted-script/',
output: 'server',
adapter: testAdapter(),
};

describe('Hoisted inline scripts in SSR', () => {
/** @type {import('./test-utils').Fixture} */
let fixture;

describe('without base path', () => {
before(async () => {
fixture = await loadFixture({
root: './fixtures/ssr-hoisted-script/',
output: 'server',
adapter: testAdapter(),
});
fixture = await loadFixture(defaultFixtureOptions);
await fixture.build();
});

it('Inlined scripts get included', async () => {
it('scripts get included', async () => {
const html = await fetchHTML(fixture, '/');
const $ = cheerioLoad(html);
expect($('script').length).to.equal(1);
});
});

describe('with base path', () => {
const base = '/hello';

before(async () => {
fixture = await loadFixture({
...defaultFixtureOptions,
base,
});
await fixture.build();
});

it('Inlined scripts get included without base path in the script', async () => {
const html = await fetchHTML(fixture, '/hello/');
const $ = cheerioLoad(html);
expect($('script').html()).to.equal('console.log("hello world");\n');
});
});
});

describe('Hoisted scripts in SSR with base path', () => {
describe('Hoisted external scripts in SSR', () => {
/** @type {import('./test-utils').Fixture} */
let fixture;
const base = '/hello';

before(async () => {
fixture = await loadFixture({
root: './fixtures/ssr-hoisted-script/',
output: 'server',
adapter: testAdapter(),
base,
describe('without base path', () => {
before(async () => {
fixture = await loadFixture({
...defaultFixtureOptions,
vite: {
build: {
assetsInlineLimit: 0,
},
},
});
await fixture.build();
});

it('script has correct path', async () => {
const html = await fetchHTML(fixture, '/');
const $ = cheerioLoad(html);
expect($('script').attr('src')).to.match(/^\/_astro\/hoisted\..{8}\.js$/);
});
});

describe('with base path', () => {
before(async () => {
fixture = await loadFixture({
...defaultFixtureOptions,
vite: {
build: {
assetsInlineLimit: 0,
},
},
base: '/hello',
});
await fixture.build();
});

it('script has correct path', async () => {
const html = await fetchHTML(fixture, '/hello/');
const $ = cheerioLoad(html);
expect($('script').attr('src')).to.match(/^\/hello\/_astro\/hoisted\..{8}\.js$/);
});
});

describe('with assetsPrefix', () => {
before(async () => {
fixture = await loadFixture({
...defaultFixtureOptions,
vite: {
build: {
assetsInlineLimit: 0,
},
},
build: {
assetsPrefix: 'https://cdn.example.com',
},
});
await fixture.build();
});

it('script has correct path', async () => {
const html = await fetchHTML(fixture, '/');
const $ = cheerioLoad(html);
expect($('script').attr('src')).to.match(
/^https:\/\/cdn\.example\.com\/_astro\/hoisted\..{8}\.js$/
);
});
await fixture.build();
});

it('Inlined scripts get included without base path in the script', async () => {
const html = await fetchHTML(fixture, '/hello/');
const $ = cheerioLoad(html);
expect($('script').html()).to.equal('console.log("hello world");\n');
describe('with custom rollup output file names', () => {
before(async () => {
fixture = await loadFixture({
...defaultFixtureOptions,
vite: {
build: {
assetsInlineLimit: 0,
rollupOptions: {
output: {
entryFileNames: 'assets/entry.[hash].mjs',
chunkFileNames: 'assets/chunks/chunk.[hash].mjs',
assetFileNames: 'assets/asset.[hash][extname]',
},
},
},
},
});
await fixture.build();
});

it('script has correct path', async () => {
const html = await fetchHTML(fixture, '/');
const $ = cheerioLoad(html);
expect($('script').attr('src')).to.match(/^\/assets\/entry\..{8}\.mjs$/);
});
});

describe('with custom rollup output file names and base', () => {
before(async () => {
fixture = await loadFixture({
...defaultFixtureOptions,
vite: {
build: {
assetsInlineLimit: 0,
rollupOptions: {
output: {
entryFileNames: 'assets/entry.[hash].mjs',
chunkFileNames: 'assets/chunks/chunk.[hash].mjs',
assetFileNames: 'assets/asset.[hash][extname]',
},
},
},
},
base: '/hello',
});
await fixture.build();
});

it('script has correct path', async () => {
const html = await fetchHTML(fixture, '/hello/');
const $ = cheerioLoad(html);
expect($('script').attr('src')).to.match(/^\/hello\/assets\/entry\..{8}\.mjs$/);
});
});

describe('with custom rollup output file names and assetsPrefix', () => {
before(async () => {
fixture = await loadFixture({
...defaultFixtureOptions,
vite: {
build: {
assetsInlineLimit: 0,
rollupOptions: {
output: {
entryFileNames: 'assets/entry.[hash].mjs',
chunkFileNames: 'assets/chunks/chunk.[hash].mjs',
assetFileNames: 'assets/asset.[hash][extname]',
},
},
},
},
build: {
assetsPrefix: 'https://cdn.example.com',
},
});
await fixture.build();
});

it('script has correct path', async () => {
const html = await fetchHTML(fixture, '/');
const $ = cheerioLoad(html);
expect($('script').attr('src')).to.match(
/^https:\/\/cdn\.example\.com\/assets\/entry\..{8}\.mjs$/
);
});
});
});

0 comments on commit 354a62c

Please sign in to comment.