From 4fb00544487859112c24c70827ed8b178fde05bc Mon Sep 17 00:00:00 2001 From: imccyu <276526105+imccyu@users.noreply.github.com> Date: Tue, 20 Oct 2026 04:00:23 +0800 Subject: [PATCH] fix(client): rewrite image-set urls and keep the route fixture valid on Windows --- .../tests/package-assets.client.spec.ts | 7 ++-- packages/client/tsdown.client.ts | 33 ++++++++++++------- scripts/client-bundle-css.spec.ts | 19 ++++++++++- scripts/verify-client-packages.spec.ts | 3 +- scripts/verify-client-packages.ts | 7 ++++ 5 files changed, 53 insertions(+), 16 deletions(-) diff --git a/packages/client/modules/tests/package-assets.client.spec.ts b/packages/client/modules/tests/package-assets.client.spec.ts index 006b4efea6..acbb8889b7 100644 --- a/packages/client/modules/tests/package-assets.client.spec.ts +++ b/packages/client/modules/tests/package-assets.client.spec.ts @@ -81,7 +81,8 @@ function writeFixture(): string { writeFile(join(assets, 'icon.svg'), SVG) writeFile(join(assets, 'photo.png'), PNG) writeFile(join(assets, 'nested', 'a b.webp'), WEBP) - writeFile(join(assets, "whale(2x)!'*.png"), PNG) + // `*` is covered by the reference unit test; Windows cannot create it on disk. + writeFile(join(assets, "whale(2x)!'.png"), PNG) writeFile(join(assets, 'notes.txt'), 'text\n') writeFile(join(assets, 'folder.png', 'inner.png'), PNG) writeFile(join(assets, 'private', 'hidden.png'), PNG) @@ -207,8 +208,8 @@ describe('package asset route', () => { const nested = await both(assetUrl(ASSETS_ID, 'nested/a b.webp')) expect([nested.status, nested.headers.etag, nested.body]).toEqual([200, '"nested/a%20b.webp"', WEBP]) // The reference encodes the characters an unquoted CSS url() rejects; the route decodes them back. - const special = await both(assetUrl(ASSETS_ID, "whale(2x)!'*.png")) - expect([special.status, special.body]).toEqual([200, PNG]) + const special = await both(assetUrl(ASSETS_ID, "whale(2x)!'.png")) + expect([special.status, special.headers.etag, special.body]).toEqual([200, '"whale%282x%29%21%27.png"', PNG]) // The revision lives in the emitted name; a query does not select content. expect((await both(`${assetUrl(ASSETS_ID, 'photo.png')}?rev=anything`)).body).toEqual(PNG) }) diff --git a/packages/client/tsdown.client.ts b/packages/client/tsdown.client.ts index cfe07dbae6..5d29fdecdb 100644 --- a/packages/client/tsdown.client.ts +++ b/packages/client/tsdown.client.ts @@ -23,7 +23,7 @@ import { createRequire, isBuiltin } from 'node:module' import { basename, dirname, isAbsolute, join, posix, relative, resolve as resolvePath, sep } from 'node:path' import { fileURLToPath } from 'node:url' import { Rolldown, type TsdownPlugin, type UserConfig } from 'tsdown' -import { transform, type CustomAtRules, type Visitor } from 'lightningcss' +import { transform, type CustomAtRules, type Url, type Visitor } from 'lightningcss' import { PACKAGE_ASSET_EXPORT, PACKAGE_ASSET_EXPORT_TARGET, @@ -530,9 +530,9 @@ function packageAssetCall(reference: string): string { * reference the `url()` rewrite inserted through the resolver call. The rewrite writes * a placeholder of letters, digits, and underscores, which lightningcss never escapes, * with a per-stylesheet random part that stylesheet text cannot contain by chance. - * @param css - stylesheet text after the rewrite. * A placeholder may appear several times or not at all, as when minification copies * or drops a declaration. + * @param css - stylesheet text after the rewrite. * @param references - reference of each placeholder the rewrite inserted into this stylesheet. * @returns a string literal, or a concatenation of literals and resolver calls. */ @@ -663,18 +663,29 @@ function packageAssets(manifest: PackageManifest, packageDir: string): PackageAs const nonce = randomBytes(6).toString('hex') if (pathBelow(realDirectory(sourceDir), realpathSync(stylesheet)) === undefined) return { options: {}, references: inserted } const base = dirname(stylesheet) + const rewrite = (url: Url): Url | undefined => { + // Empty values, fragments, root-relative paths, schemes such as `data:`, and placeholders name no package file. + if (url.url === '' || url.url.startsWith('#') || url.url.startsWith('/') || URL_SCHEME.test(url.url)) return undefined + if (url.url.startsWith('__dshPackageAsset_')) return undefined + if (/[#?]/u.test(url.url)) { + throw new Error(`tsdown: ${name}: ${stylesheet} references ${JSON.stringify(url.url)}; a package asset url() takes no fragment or query`) + } + const placeholder = `__dshPackageAsset_${nonce}_${String(inserted.size)}__` + inserted.set(placeholder, reference(context, resolvePath(base, url.url))) + return { ...url, url: placeholder } + } return { options: { visitor: { - Url(url) { - // Empty values, fragments, root-relative paths, and schemes such as `data:` name no package file. - if (url.url === '' || url.url.startsWith('#') || url.url.startsWith('/') || URL_SCHEME.test(url.url)) return - if (/[#?]/u.test(url.url)) { - throw new Error(`tsdown: ${name}: ${stylesheet} references ${JSON.stringify(url.url)}; a package asset url() takes no fragment or query`) - } - const placeholder = `__dshPackageAsset_${nonce}_${String(inserted.size)}__` - inserted.set(placeholder, reference(context, resolvePath(base, url.url))) - return { ...url, url: placeholder } + Url: url => rewrite(url), + // lightningcss does not visit the urls inside image-set(), so its options are rewritten here. + Image(image) { + if (image.type !== 'image-set') return undefined + const options = image.value.options.map((option) => { + const url = option.image.type === 'url' ? rewrite(option.image.value) : undefined + return url === undefined ? option : { ...option, image: { type: 'url' as const, value: url } } + }) + return { ...image, value: { ...image.value, options } } }, }, }, diff --git a/scripts/client-bundle-css.spec.ts b/scripts/client-bundle-css.spec.ts index f053ea3f9b..ac34dc4603 100644 --- a/scripts/client-bundle-css.spec.ts +++ b/scripts/client-bundle-css.spec.ts @@ -291,7 +291,7 @@ describe('client bundle package asset imports', () => { .toThrow(`${join(root, path)} has a node_modules path segment or one containing %, #, ?, \\ or NUL, which the asset route cannot serve; rename it`) }) - it('resolves every copy of a reference and tolerates one minification drops', async () => { + it('resolves every copy of a reference and tolerates a minifier dropping one', async () => { const root = await assetPackage({ 'src/client/assets/whale.png': WHALE, 'src/client/assets/old.png': WHALE, @@ -302,9 +302,26 @@ describe('client bundle package asset imports', () => { const css = stylesheetText(output, reference => `resolved:${reference}`) expect(css.split(`url(resolved:${WHALE_URL})`)).toHaveLength(3) + // lightningcss keeps only the last background, so the placeholder of old.png is missing from the text. + expect(css).not.toContain('old-') expect(css).not.toContain('__dshPackageAsset') }) + it('rewrites the urls inside image-set()', async () => { + const root = await assetPackage({ + 'src/client/assets/whale.png': WHALE, + 'src/client/theme.css': '.a { background-image: image-set(url(./assets/whale.png) 1x, url("data:image/png;base64,AA==") 2x); }', + }) + const context = recordingContext() + + const output = await loadImport(browserPlugins(root)('dsh-css-global-inline'), context, './theme.css', join(root, 'src/client/index.ts')) + + const css = stylesheetText(output, reference => `resolved:${reference}`) + expect(css).toContain(`resolved:${WHALE_URL}`) + expect(css).toContain('data:image/png;base64,AA==') + expect(context.emitted.map(file => file.fileName)).toEqual([`assets/whale-${WHALE_REV}.png`]) + }) + it('keeps literal placeholder-like text and resolves only the references the rewrite inserted', async () => { const root = await assetPackage({ 'src/client/assets/whale.png': WHALE, diff --git a/scripts/verify-client-packages.spec.ts b/scripts/verify-client-packages.spec.ts index 84a543d862..05fd6cc574 100644 --- a/scripts/verify-client-packages.spec.ts +++ b/scripts/verify-client-packages.spec.ts @@ -331,7 +331,8 @@ describe('package assets', () => { '.network { background: url(//cdn.example.com/x.png); }', ".remote { background: url('https://example.com/x.png'); }", "@supports (mask-image: url('')) { .nested { mask: url(./nested.svg); } }", - ].join('\n'))).toEqual(['./x.png', '../assets/x.png', './bare.svg', './nested.svg']) + ".set { background-image: image-set(url('./set@1x.png') 1x, url('data:image/png;base64,AA==') 2x); }", + ].join('\n'))).toEqual(['./x.png', '../assets/x.png', './bare.svg', './nested.svg', './set@1x.png']) }) it('rejects an unparsable stylesheet with its path', () => { diff --git a/scripts/verify-client-packages.ts b/scripts/verify-client-packages.ts index a32bf85c46..9cbbd4981b 100644 --- a/scripts/verify-client-packages.ts +++ b/scripts/verify-client-packages.ts @@ -173,6 +173,13 @@ export function collectStylesheetAssetUrls(path: string, source: string): string Url(url) { if (isRelativeStylesheetUrl(url.url)) urls.push(url.url) }, + // lightningcss does not visit the urls inside image-set(), so its options are read here. + Image(image) { + if (image.type !== 'image-set') return + for (const option of image.value.options) { + if (option.image.type === 'url' && isRelativeStylesheetUrl(option.image.value.url)) urls.push(option.image.value.url) + } + }, }, }) } catch (error) {