From 8acd6a316bbcc82ae16ecb0b6ef123792d66e322 Mon Sep 17 00:00:00 2001 From: James Ross Date: Sun, 18 Feb 2024 18:28:40 +0000 Subject: [PATCH 1/8] fix: string assetsInlineLimit --- .changeset/late-bears-collect.md | 5 ++ packages/astro/src/core/build/plugins/util.ts | 4 +- packages/astro/test/css-assets.test.js | 46 +++++++++++++++++++ 3 files changed, 53 insertions(+), 2 deletions(-) create mode 100644 .changeset/late-bears-collect.md diff --git a/.changeset/late-bears-collect.md b/.changeset/late-bears-collect.md new file mode 100644 index 000000000000..d2746d961825 --- /dev/null +++ b/.changeset/late-bears-collect.md @@ -0,0 +1,5 @@ +--- +"astro": patch +--- + +Fixes a minor regression from 4.3.x when the vite.build.assetsInlineLimit configuration option was set to a string. Astro now automatically casts this to a number to match the Vite behaviour. diff --git a/packages/astro/src/core/build/plugins/util.ts b/packages/astro/src/core/build/plugins/util.ts index 7297a1f6f26b..a114ab719fb3 100644 --- a/packages/astro/src/core/build/plugins/util.ts +++ b/packages/astro/src/core/build/plugins/util.ts @@ -75,8 +75,8 @@ export function shouldInlineAsset( assetPath: string, assetsInlineLimit: NonNullable ) { - if (typeof assetsInlineLimit === 'number') { - return Buffer.byteLength(assetContent) < assetsInlineLimit; + if (typeof assetsInlineLimit === 'number' || typeof assetsInlineLimit === 'string') { + return Buffer.byteLength(assetContent) < Number(assetsInlineLimit); } const result = assetsInlineLimit(assetPath, Buffer.from(assetContent)); diff --git a/packages/astro/test/css-assets.test.js b/packages/astro/test/css-assets.test.js index 386d8b5751e1..ffdf9979f691 100644 --- a/packages/astro/test/css-assets.test.js +++ b/packages/astro/test/css-assets.test.js @@ -48,3 +48,49 @@ describe('Assets in CSS', () => { assert.equal(getAllMatches(/font-face/g, css), 1); }); }); + +describe('Assets in CSS - string assetsInlineLimit', () => { + let fixture; + + before(async () => { + fixture = await loadFixture({ + root: './fixtures/css-assets/', + vite: { + build: { + assetsInlineLimit: '0', + }, + }, + }); + await fixture.build(); + }); + + function getAllMatches(re, text) { + let count = 0; + while (re.exec(text) !== null) { + ++count; + } + return count; + } + + async function getCSSForPage(pathname) { + const html = await fixture.readFile(pathname); + const $ = cheerio.load(html); + const cssPath = $('link').attr('href'); + const css = await fixture.readFile(cssPath); + return css; + } + + it('Bundled CSS does not have __VITE_ASSET__', async () => { + let css = await getCSSForPage('/one/index.html'); + assert.equal(css.includes('__VITE_ASSET__'), false); + css = await getCSSForPage('/two/index.html'); + assert.equal(css.includes('__VITE_ASSET__'), false); + }); + + it('Pages contain only their own CSS', async () => { + let css = await getCSSForPage('/one/index.html'); + assert.equal(getAllMatches(/font-face/g, css), 1); + css = await getCSSForPage('/two/index.html'); + assert.equal(getAllMatches(/font-face/g, css), 1); + }); +}); From 3d38e7fc512007f44ec1f3a23f61f4a30676d427 Mon Sep 17 00:00:00 2001 From: James Ross Date: Mon, 19 Feb 2024 14:19:29 +0000 Subject: [PATCH 2/8] fix: better handle NaN values for `assetsInlineLimit` --- packages/astro/src/core/build/plugins/util.ts | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/packages/astro/src/core/build/plugins/util.ts b/packages/astro/src/core/build/plugins/util.ts index a114ab719fb3..cd269b3ead0d 100644 --- a/packages/astro/src/core/build/plugins/util.ts +++ b/packages/astro/src/core/build/plugins/util.ts @@ -75,14 +75,23 @@ export function shouldInlineAsset( assetPath: string, assetsInlineLimit: NonNullable ) { - if (typeof assetsInlineLimit === 'number' || typeof assetsInlineLimit === 'string') { - return Buffer.byteLength(assetContent) < Number(assetsInlineLimit); + if(typeof(assetsInlineLimit) === 'function'){ + const result = assetsInlineLimit(assetPath, Buffer.from(assetContent)); + if (result != null) { + return result; + } + } + let limit = 4096; // Fallback to 4096kb by default (same as Vite) + if(typeof assetsInlineLimit === 'number'){ + limit = assetsInlineLimit; + }else if (typeof assetsInlineLimit === 'string') { + // TODO: stop accepting strings for assetsInlineLimit, see https://github.com/withastro/astro/pull/10154 + limit = Number(assetsInlineLimit); } - const result = assetsInlineLimit(assetPath, Buffer.from(assetContent)); - if (result != null) { - return result; + if(Number.isNaN(limit)){ + limit = 4096; } - return Buffer.byteLength(assetContent) < 4096; // Fallback to 4096kb by default (same as Vite) + return Buffer.byteLength(assetContent) < limit; } From 6d81776e0ecbe7ceaef12ecb09c4d6825ab8cc13 Mon Sep 17 00:00:00 2001 From: James Ross Date: Mon, 19 Feb 2024 19:53:01 +0000 Subject: [PATCH 3/8] chore: prettier --- packages/astro/src/core/build/plugins/util.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/astro/src/core/build/plugins/util.ts b/packages/astro/src/core/build/plugins/util.ts index cd269b3ead0d..81768493e0d2 100644 --- a/packages/astro/src/core/build/plugins/util.ts +++ b/packages/astro/src/core/build/plugins/util.ts @@ -75,21 +75,21 @@ export function shouldInlineAsset( assetPath: string, assetsInlineLimit: NonNullable ) { - if(typeof(assetsInlineLimit) === 'function'){ + if (typeof assetsInlineLimit === 'function') { const result = assetsInlineLimit(assetPath, Buffer.from(assetContent)); if (result != null) { return result; } } let limit = 4096; // Fallback to 4096kb by default (same as Vite) - if(typeof assetsInlineLimit === 'number'){ + if (typeof assetsInlineLimit === 'number') { limit = assetsInlineLimit; - }else if (typeof assetsInlineLimit === 'string') { + } else if (typeof assetsInlineLimit === 'string') { // TODO: stop accepting strings for assetsInlineLimit, see https://github.com/withastro/astro/pull/10154 limit = Number(assetsInlineLimit); } - if(Number.isNaN(limit)){ + if (Number.isNaN(limit)) { limit = 4096; } From a76662bee8c2a1fa97b62097039df2dcdb29687f Mon Sep 17 00:00:00 2001 From: James Ross Date: Tue, 20 Feb 2024 17:03:43 +0000 Subject: [PATCH 4/8] chore: simplify for requested changes --- packages/astro/src/core/build/plugins/util.ts | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/packages/astro/src/core/build/plugins/util.ts b/packages/astro/src/core/build/plugins/util.ts index 81768493e0d2..2b12490c77d6 100644 --- a/packages/astro/src/core/build/plugins/util.ts +++ b/packages/astro/src/core/build/plugins/util.ts @@ -81,17 +81,9 @@ export function shouldInlineAsset( return result; } } - let limit = 4096; // Fallback to 4096kb by default (same as Vite) - if (typeof assetsInlineLimit === 'number') { - limit = assetsInlineLimit; - } else if (typeof assetsInlineLimit === 'string') { - // TODO: stop accepting strings for assetsInlineLimit, see https://github.com/withastro/astro/pull/10154 - limit = Number(assetsInlineLimit); + if (typeof assetsInlineLimit === 'number' || typeof assetsInlineLimit === 'string') { + return Buffer.byteLength(assetContent) < Number(assetsInlineLimit); } - if (Number.isNaN(limit)) { - limit = 4096; - } - - return Buffer.byteLength(assetContent) < limit; + return Buffer.byteLength(assetContent) < 4096; // Fallback to 4096kb by default (same as Vite) } From 5aa199fd951d4ff165efaf0dc547657b42ea41ea Mon Sep 17 00:00:00 2001 From: James Ross Date: Tue, 20 Feb 2024 17:04:43 +0000 Subject: [PATCH 5/8] chore: update changeset --- .changeset/late-bears-collect.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/late-bears-collect.md b/.changeset/late-bears-collect.md index d2746d961825..3fe2bc8051a0 100644 --- a/.changeset/late-bears-collect.md +++ b/.changeset/late-bears-collect.md @@ -2,4 +2,4 @@ "astro": patch --- -Fixes a minor regression from 4.3.x when the vite.build.assetsInlineLimit configuration option was set to a string. Astro now automatically casts this to a number to match the Vite behaviour. +Improves runtime type-checking with the vite.build.assetsInlineLimit configuration option, which could result in errors when passing invalid options like strings. Astro now automatically casts this to a number to match the Vite behaviour. From 34aad9fa718581cdb41127b38a28097700adfa81 Mon Sep 17 00:00:00 2001 From: James Ross Date: Tue, 20 Feb 2024 17:09:31 +0000 Subject: [PATCH 6/8] chore: remove tests --- packages/astro/test/css-assets.test.js | 46 -------------------------- 1 file changed, 46 deletions(-) diff --git a/packages/astro/test/css-assets.test.js b/packages/astro/test/css-assets.test.js index ffdf9979f691..386d8b5751e1 100644 --- a/packages/astro/test/css-assets.test.js +++ b/packages/astro/test/css-assets.test.js @@ -48,49 +48,3 @@ describe('Assets in CSS', () => { assert.equal(getAllMatches(/font-face/g, css), 1); }); }); - -describe('Assets in CSS - string assetsInlineLimit', () => { - let fixture; - - before(async () => { - fixture = await loadFixture({ - root: './fixtures/css-assets/', - vite: { - build: { - assetsInlineLimit: '0', - }, - }, - }); - await fixture.build(); - }); - - function getAllMatches(re, text) { - let count = 0; - while (re.exec(text) !== null) { - ++count; - } - return count; - } - - async function getCSSForPage(pathname) { - const html = await fixture.readFile(pathname); - const $ = cheerio.load(html); - const cssPath = $('link').attr('href'); - const css = await fixture.readFile(cssPath); - return css; - } - - it('Bundled CSS does not have __VITE_ASSET__', async () => { - let css = await getCSSForPage('/one/index.html'); - assert.equal(css.includes('__VITE_ASSET__'), false); - css = await getCSSForPage('/two/index.html'); - assert.equal(css.includes('__VITE_ASSET__'), false); - }); - - it('Pages contain only their own CSS', async () => { - let css = await getCSSForPage('/one/index.html'); - assert.equal(getAllMatches(/font-face/g, css), 1); - css = await getCSSForPage('/two/index.html'); - assert.equal(getAllMatches(/font-face/g, css), 1); - }); -}); From 07cd3f151c7d26b0d55360a24420d1348db5f044 Mon Sep 17 00:00:00 2001 From: James Ross Date: Fri, 23 Feb 2024 15:01:08 +0000 Subject: [PATCH 7/8] chore: simplify function --- packages/astro/src/core/build/plugins/util.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/astro/src/core/build/plugins/util.ts b/packages/astro/src/core/build/plugins/util.ts index 2b12490c77d6..07b18a8870ce 100644 --- a/packages/astro/src/core/build/plugins/util.ts +++ b/packages/astro/src/core/build/plugins/util.ts @@ -79,11 +79,9 @@ export function shouldInlineAsset( const result = assetsInlineLimit(assetPath, Buffer.from(assetContent)); if (result != null) { return result; + } else { + return Buffer.byteLength(assetContent) < 4096; // Fallback to 4096kb by default (same as Vite) } } - if (typeof assetsInlineLimit === 'number' || typeof assetsInlineLimit === 'string') { - return Buffer.byteLength(assetContent) < Number(assetsInlineLimit); - } - - return Buffer.byteLength(assetContent) < 4096; // Fallback to 4096kb by default (same as Vite) + return Buffer.byteLength(assetContent) < Number(assetsInlineLimit); } From d721ccc4c6fa06f845e0d4e20713ab64fac03c08 Mon Sep 17 00:00:00 2001 From: Arsh <69170106+lilnasy@users.noreply.github.com> Date: Mon, 26 Feb 2024 20:12:27 +0530 Subject: [PATCH 8/8] Apply suggestions from code review --- .changeset/late-bears-collect.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/late-bears-collect.md b/.changeset/late-bears-collect.md index 3fe2bc8051a0..f2bcd193fd7a 100644 --- a/.changeset/late-bears-collect.md +++ b/.changeset/late-bears-collect.md @@ -2,4 +2,4 @@ "astro": patch --- -Improves runtime type-checking with the vite.build.assetsInlineLimit configuration option, which could result in errors when passing invalid options like strings. Astro now automatically casts this to a number to match the Vite behaviour. +Fixes an issue where `config.vite.build.assetsInlineLimit` could not be set as a function.