From 8ce1874b9e97ae19a6afeca91d75fcaf5c7fa734 Mon Sep 17 00:00:00 2001 From: Alex Miller Date: Wed, 1 Feb 2023 11:25:54 +1300 Subject: [PATCH 1/3] fix: public asset urls being treated as paths --- packages/vite/src/node/plugins/html.ts | 11 +++++++---- packages/vite/src/node/utils.ts | 9 +++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/vite/src/node/plugins/html.ts b/packages/vite/src/node/plugins/html.ts index 9737dcb3a9e9b7..6b2bb3a46b82ae 100644 --- a/packages/vite/src/node/plugins/html.ts +++ b/packages/vite/src/node/plugins/html.ts @@ -18,6 +18,7 @@ import { getHash, isDataUrl, isExternalUrl, + isUrl, normalizePath, processSrcSet, } from '../utils' @@ -811,11 +812,13 @@ export function buildHtmlPlugin(config: ResolvedConfig): Plugin { }) result = result.replace(publicAssetUrlRE, (_, fileHash) => { - return normalizePath( - toOutputPublicAssetFilePath( - getPublicAssetFilename(fileHash, config)!, - ), + const publicAssetPath = toOutputPublicAssetFilePath( + getPublicAssetFilename(fileHash, config)!, ) + + return isUrl(publicAssetPath) + ? publicAssetPath + : normalizePath(publicAssetPath) }) if (chunk && canInlineEntry) { diff --git a/packages/vite/src/node/utils.ts b/packages/vite/src/node/utils.ts index c12e429c51529d..1337fda226f526 100644 --- a/packages/vite/src/node/utils.ts +++ b/packages/vite/src/node/utils.ts @@ -217,6 +217,15 @@ function testCaseInsensitiveFS() { return fs.existsSync(CLIENT_ENTRY.replace('client.mjs', 'cLiEnT.mjs')) } +export function isUrl(path: string): boolean { + try { + new URL(path) + return true + } catch { + return false + } +} + export const isCaseInsensitiveFS = testCaseInsensitiveFS() export const isWindows = os.platform() === 'win32' From a5e5dd4653ad5a89002ef5d1cd8a71d0f5e40ee1 Mon Sep 17 00:00:00 2001 From: Alex Miller Date: Thu, 2 Feb 2023 12:58:07 +1300 Subject: [PATCH 2/3] test: Add url asset tests, based on relative --- .../url-base/url-base-assets.spec.ts | 235 ++++++++++++++++++ .../assets/__tests__/url-base/vite.config.js | 1 + playground/assets/package.json | 5 +- playground/assets/vite.config-url-base.js | 28 +++ 4 files changed, 268 insertions(+), 1 deletion(-) create mode 100644 playground/assets/__tests__/url-base/url-base-assets.spec.ts create mode 100644 playground/assets/__tests__/url-base/vite.config.js create mode 100644 playground/assets/vite.config-url-base.js diff --git a/playground/assets/__tests__/url-base/url-base-assets.spec.ts b/playground/assets/__tests__/url-base/url-base-assets.spec.ts new file mode 100644 index 00000000000000..775f8c15caddc2 --- /dev/null +++ b/playground/assets/__tests__/url-base/url-base-assets.spec.ts @@ -0,0 +1,235 @@ +import { beforeAll, describe, expect, test } from 'vitest' +import { + browserLogs, + findAssetFile, + getBg, + getColor, + isBuild, + page, + viteConfig, +} from '~utils' + +const urlAssetMatch = isBuild + ? /http:\/\/localhost:4173\/other-assets\/asset-\w{8}\.png/ + : '/nested/asset.png' + +const iconMatch = '/icon.png' + +const absoluteIconMatch = isBuild + ? /http:\/\/localhost:4173\/.*\/icon-\w{8}\.png/ + : '/nested/icon.png' + +const absolutePublicIconMatch = isBuild + ? /http:\/\/localhost:4173\/icon\.png/ + : '/icon.png' + +test('should have no 404s', () => { + browserLogs.forEach((msg) => { + expect(msg).not.toMatch('404') + }) +}) + +describe('raw references from /public', () => { + test('load raw js from /public', async () => { + expect(await page.textContent('.raw-js')).toMatch('[success]') + }) + + test('load raw css from /public', async () => { + expect(await getColor('.raw-css')).toBe('red') + }) +}) + +test('import-expression from simple script', async () => { + expect(await page.textContent('.import-expression')).toMatch( + '[success][success]', + ) +}) + +describe('asset imports from js', () => { + test('relative', async () => { + expect(await page.textContent('.asset-import-relative')).toMatch( + urlAssetMatch, + ) + }) + + test('absolute', async () => { + expect(await page.textContent('.asset-import-absolute')).toMatch( + urlAssetMatch, + ) + }) + + test('from /public', async () => { + expect(await page.textContent('.public-import')).toMatch( + absolutePublicIconMatch, + ) + }) +}) + +describe('css url() references', () => { + test('fonts', async () => { + expect( + await page.evaluate(() => document.fonts.check('700 32px Inter')), + ).toBe(true) + }) + + test('relative', async () => { + const bg = await getBg('.css-url-relative') + expect(bg).toMatch(urlAssetMatch) + }) + + test('image-set relative', async () => { + const imageSet = await getBg('.css-image-set-relative') + imageSet.split(', ').forEach((s) => { + expect(s).toMatch(urlAssetMatch) + }) + }) + + test('image-set without the url() call', async () => { + const imageSet = await getBg('.css-image-set-without-url-call') + imageSet.split(', ').forEach((s) => { + expect(s).toMatch(urlAssetMatch) + }) + }) + + test('image-set with var', async () => { + const imageSet = await getBg('.css-image-set-with-var') + imageSet.split(', ').forEach((s) => { + expect(s).toMatch(urlAssetMatch) + }) + }) + + test('image-set with mix', async () => { + const imageSet = await getBg('.css-image-set-mix-url-var') + imageSet.split(', ').forEach((s) => { + expect(s).toMatch(urlAssetMatch) + }) + }) + + test('relative in @import', async () => { + expect(await getBg('.css-url-relative-at-imported')).toMatch(urlAssetMatch) + }) + + test('absolute', async () => { + expect(await getBg('.css-url-absolute')).toMatch(urlAssetMatch) + }) + + test('from /public', async () => { + expect(await getBg('.css-url-public')).toMatch(iconMatch) + }) + + test('multiple urls on the same line', async () => { + const bg = await getBg('.css-url-same-line') + expect(bg).toMatch(urlAssetMatch) + expect(bg).toMatch(iconMatch) + }) + + test('aliased', async () => { + const bg = await getBg('.css-url-aliased') + expect(bg).toMatch(urlAssetMatch) + }) +}) + +describe.runIf(isBuild)('index.css URLs', () => { + let css: string + beforeAll(() => { + const base = viteConfig ? viteConfig?.testConfig?.baseRoute : '' + css = findAssetFile(/index.*\.css$/, base, 'other-assets') + }) + + test('use base URL for asset URL', () => { + expect(css).toMatch(urlAssetMatch) + }) + + test('preserve postfix query/hash', () => { + expect(css).toMatch('woff2?#iefix') + }) +}) + +describe('image', () => { + test('srcset', async () => { + const img = await page.$('.img-src-set') + const srcset = await img.getAttribute('srcset') + srcset.split(', ').forEach((s) => { + expect(s).toMatch( + isBuild + ? /other-assets\/asset-\w{8}\.png \dx/ + : /\.\/nested\/asset\.png \dx/, + ) + }) + }) +}) + +describe('svg fragments', () => { + // 404 is checked already, so here we just ensure the urls end with #fragment + test('img url', async () => { + const img = await page.$('.svg-frag-img') + expect(await img.getAttribute('src')).toMatch(/svg#icon-clock-view$/) + }) + + test('via css url()', async () => { + const bg = await page.evaluate( + () => getComputedStyle(document.querySelector('.icon')).backgroundImage, + ) + expect(bg).toMatch(/svg#icon-clock-view"\)$/) + }) + + test('from js import', async () => { + const img = await page.$('.svg-frag-import') + expect(await img.getAttribute('src')).toMatch(/svg#icon-heart-view$/) + }) +}) + +test('?raw import', async () => { + expect(await page.textContent('.raw')).toMatch('SVG') +}) + +test('?url import', async () => { + expect(await page.textContent('.url')).toMatch( + isBuild + ? /http:\/\/localhost:4173\/other-assets\/foo-\w{8}\.js/ + : '/foo.js', + ) +}) + +test('?url import on css', async () => { + const txt = await page.textContent('.url-css') + expect(txt).toMatch( + isBuild + ? /http:\/\/localhost:4173\/other-assets\/icons-\w{8}\.css/ + : '/css/icons.css', + ) +}) + +test('new URL(..., import.meta.url)', async () => { + expect(await page.textContent('.import-meta-url')).toMatch(urlAssetMatch) +}) + +test('new URL(`${dynamic}`, import.meta.url)', async () => { + const dynamic1 = await page.textContent('.dynamic-import-meta-url-1') + expect(dynamic1).toMatch(absoluteIconMatch) + const dynamic2 = await page.textContent('.dynamic-import-meta-url-2') + expect(dynamic2).toMatch(urlAssetMatch) +}) + +test('new URL(`non-existent`, import.meta.url)', async () => { + expect(await page.textContent('.non-existent-import-meta-url')).toMatch( + '/non-existent', + ) +}) + +test('inline style test', async () => { + expect(await getBg('.inline-style')).toMatch(urlAssetMatch) + expect(await getBg('.style-url-assets')).toMatch(urlAssetMatch) +}) + +test('html import word boundary', async () => { + expect(await page.textContent('.obj-import-express')).toMatch( + 'ignore object import prop', + ) + expect(await page.textContent('.string-import-express')).toMatch('no load') +}) + +test('relative path in html asset', async () => { + expect(await page.textContent('.relative-js')).toMatch('hello') + expect(await getColor('.relative-css')).toMatch('red') +}) diff --git a/playground/assets/__tests__/url-base/vite.config.js b/playground/assets/__tests__/url-base/vite.config.js new file mode 100644 index 00000000000000..b096ea1d30caad --- /dev/null +++ b/playground/assets/__tests__/url-base/vite.config.js @@ -0,0 +1 @@ +module.exports = require('../../vite.config-url-base') diff --git a/playground/assets/package.json b/playground/assets/package.json index beec899e36bd68..47e127261142e7 100644 --- a/playground/assets/package.json +++ b/playground/assets/package.json @@ -12,6 +12,9 @@ "preview:relative-base": "vite --config ./vite.config-relative-base.js preview", "dev:runtime-base": "vite --config ./vite.config-runtime-base.js dev", "build:runtime-base": "vite --config ./vite.config-runtime-base.js build", - "preview:runtime-base": "vite --config ./vite.config-runtime-base.js preview" + "preview:runtime-base": "vite --config ./vite.config-runtime-base.js preview", + "dev:url-base": "vite --config ./vite.config-url-base.js dev", + "build:url-base": "vite --config ./vite.config-url-base.js build", + "preview:url-base": "vite --config ./vite.config-url-base.js preview" } } diff --git a/playground/assets/vite.config-url-base.js b/playground/assets/vite.config-url-base.js new file mode 100644 index 00000000000000..3632526fc3c21a --- /dev/null +++ b/playground/assets/vite.config-url-base.js @@ -0,0 +1,28 @@ +/** + * @type {import('vite').UserConfig} + */ + +const { DEFAULT_PREVIEW_PORT } = require('vite') + +const baseConfig = require('./vite.config.js') +module.exports = { + ...baseConfig, + base: 'http://localhost:4173/', + build: { + ...baseConfig.build, + outDir: 'dist/url-base', + watch: false, + minify: false, + assetsInlineLimit: 0, + rollupOptions: { + output: { + entryFileNames: 'entries/[name].js', + chunkFileNames: 'chunks/[name]-[hash].js', + assetFileNames: 'other-assets/[name]-[hash][extname]', + }, + }, + }, + testConfig: { + baseRoute: '/url-base/', + }, +} From 8a2b9b037dfda7c71f2ea18efd0b4eeabb7da867 Mon Sep 17 00:00:00 2001 From: Alex Miller Date: Thu, 16 Mar 2023 10:35:15 +1300 Subject: [PATCH 3/3] refactor(playground): move new test to ESM to match upstream --- .../assets/__tests__/url-base/vite.config.js | 2 +- playground/assets/vite.config-url-base.js | 14 +++++--------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/playground/assets/__tests__/url-base/vite.config.js b/playground/assets/__tests__/url-base/vite.config.js index b096ea1d30caad..4ad6a7066edad3 100644 --- a/playground/assets/__tests__/url-base/vite.config.js +++ b/playground/assets/__tests__/url-base/vite.config.js @@ -1 +1 @@ -module.exports = require('../../vite.config-url-base') +export { default } from '../../vite.config-url-base' diff --git a/playground/assets/vite.config-url-base.js b/playground/assets/vite.config-url-base.js index 3632526fc3c21a..bed30f8d1b2e73 100644 --- a/playground/assets/vite.config-url-base.js +++ b/playground/assets/vite.config-url-base.js @@ -1,17 +1,13 @@ -/** - * @type {import('vite').UserConfig} - */ +import { defineConfig } from 'vite' +import baseConfig from './vite.config.js' -const { DEFAULT_PREVIEW_PORT } = require('vite') - -const baseConfig = require('./vite.config.js') -module.exports = { +export default defineConfig({ ...baseConfig, base: 'http://localhost:4173/', build: { ...baseConfig.build, outDir: 'dist/url-base', - watch: false, + watch: null, minify: false, assetsInlineLimit: 0, rollupOptions: { @@ -25,4 +21,4 @@ module.exports = { testConfig: { baseRoute: '/url-base/', }, -} +})