Skip to content

Commit

Permalink
fix(html): skip inlining icon and manifest links (#14958)
Browse files Browse the repository at this point in the history
Co-authored-by: 翠 / green <green@sapphi.red>
  • Loading branch information
bluwy and sapphi-red committed Dec 6, 2023
1 parent 8409b66 commit 8ad81b4
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 15 deletions.
22 changes: 14 additions & 8 deletions packages/vite/src/node/plugins/asset.ts
Expand Up @@ -329,6 +329,7 @@ async function fileToBuiltUrl(
config: ResolvedConfig,
pluginContext: PluginContext,
skipPublicCheck = false,
shouldInline?: boolean,
): Promise<string> {
if (!skipPublicCheck && checkPublicFile(id, config)) {
return publicFileToBuiltUrl(id, config)
Expand All @@ -343,15 +344,18 @@ async function fileToBuiltUrl(
const file = cleanUrl(id)
const content = await fsp.readFile(file)

if (shouldInline == null) {
shouldInline =
!!config.build.lib ||
// Don't inline SVG with fragments, as they are meant to be reused
(!(file.endsWith('.svg') && id.includes('#')) &&
!file.endsWith('.html') &&
content.length < Number(config.build.assetsInlineLimit) &&
!isGitLfsPlaceholder(content))
}

let url: string
if (
config.build.lib ||
// Don't inline SVG with fragments, as they are meant to be reused
(!(file.endsWith('.svg') && id.includes('#')) &&
!file.endsWith('.html') &&
content.length < Number(config.build.assetsInlineLimit) &&
!isGitLfsPlaceholder(content))
) {
if (shouldInline) {
if (config.build.lib && isGitLfsPlaceholder(content)) {
config.logger.warn(
colors.yellow(`Inlined file ${id} was not downloaded via Git LFS`),
Expand Down Expand Up @@ -392,6 +396,7 @@ export async function urlToBuiltUrl(
importer: string,
config: ResolvedConfig,
pluginContext: PluginContext,
shouldInline?: boolean,
): Promise<string> {
if (checkPublicFile(url, config)) {
return publicFileToBuiltUrl(url, config)
Expand All @@ -406,6 +411,7 @@ export async function urlToBuiltUrl(
pluginContext,
// skip public check since we just did it above
true,
shouldInline,
)
}

Expand Down
35 changes: 32 additions & 3 deletions packages/vite/src/node/plugins/html.ts
Expand Up @@ -50,6 +50,7 @@ const inlineCSSRE = /__VITE_INLINE_CSS__([a-z\d]{8}_\d+)__/g
const inlineImportRE =
/(?<!(?<!\.\.)\.)\bimport\s*\(("(?:[^"]|(?<=\\)")*"|'(?:[^']|(?<=\\)')*')\)/dg
const htmlLangRE = /\.(?:html|htm)$/
const spaceRe = /[\t\n\f\r ]/

const importMapRE =
/[ \t]*<script[^>]*type\s*=\s*(?:"importmap"|'importmap'|importmap)[^>]*>.*?<\/script>/is
Expand Down Expand Up @@ -141,6 +142,17 @@ export const assetAttrsConfig: Record<string, string[]> = {
use: ['xlink:href', 'href'],
}

// Some `<link rel>` elements should not be inlined in build. Excluding:
// - `shortcut` : only valid for IE <9, use `icon`
// - `mask-icon` : deprecated since Safari 12 (for pinned tabs)
// - `apple-touch-icon-precomposed` : only valid for iOS <7 (for avoiding gloss effect)
const noInlineLinkRels = new Set([
'icon',
'apple-touch-icon',
'apple-touch-startup-image',
'manifest',
])

export const isAsyncScriptMap = new WeakMap<
ResolvedConfig,
Map<string, boolean>
Expand Down Expand Up @@ -386,14 +398,14 @@ export function buildHtmlPlugin(config: ResolvedConfig): Plugin {
const namedOutput = Object.keys(
config?.build?.rollupOptions?.input || {},
)
const processAssetUrl = async (url: string) => {
const processAssetUrl = async (url: string, shouldInline?: boolean) => {
if (
url !== '' && // Empty attribute
!namedOutput.includes(url) && // Direct reference to named output
!namedOutput.includes(removeLeadingSlash(url)) // Allow for absolute references as named output can't be an absolute path
) {
try {
return await urlToBuiltUrl(url, id, config, this)
return await urlToBuiltUrl(url, id, config, this, shouldInline)
} catch (e) {
if (e.code !== 'ENOENT') {
throw e
Expand Down Expand Up @@ -522,9 +534,26 @@ export function buildHtmlPlugin(config: ResolvedConfig): Plugin {
})
js += importExpression
} else {
// If the node is a link, check if it can be inlined. If not, set `shouldInline`
// to `false` to force no inline. If `undefined`, it leaves to the default heuristics.
const isNoInlineLink =
node.nodeName === 'link' &&
node.attrs.some(
(p) =>
p.name === 'rel' &&
p.value
.split(spaceRe)
.some((v) =>
noInlineLinkRels.has(v.toLowerCase()),
),
)
const shouldInline = isNoInlineLink ? false : undefined
assetUrlsPromises.push(
(async () => {
const processedUrl = await processAssetUrl(url)
const processedUrl = await processAssetUrl(
url,
shouldInline,
)
if (processedUrl !== url) {
overwriteAttrValue(
s,
Expand Down
18 changes: 14 additions & 4 deletions playground/assets/__tests__/assets.spec.ts
Expand Up @@ -223,10 +223,20 @@ describe('css url() references', () => {
const match = isBuild ? `data:image/png;base64` : `/foo/bar/nested/icon.png`
expect(await getBg('.css-url-base64-inline')).toMatch(match)
expect(await getBg('.css-url-quotes-base64-inline')).toMatch(match)
const icoMatch = isBuild ? `data:image/x-icon;base64` : `favicon.ico`
const el = await page.$(`link.ico`)
const href = await el.getAttribute('href')
expect(href).toMatch(icoMatch)
})

test('no base64 inline for icon and manifest links', async () => {
const iconEl = await page.$(`link.ico`)
const href = await iconEl.getAttribute('href')
expect(href).toMatch(
isBuild ? /\/foo\/bar\/assets\/favicon-[-\w]{8}\.ico/ : 'favicon.ico',
)

const manifestEl = await page.$(`link[rel="manifest"]`)
const manifestHref = await manifestEl.getAttribute('href')
expect(manifestHref).toMatch(
isBuild ? /\/foo\/bar\/assets\/manifest-[-\w]{8}\.json/ : 'manifest.json',
)
})

test('multiple urls on the same line', async () => {
Expand Down
1 change: 1 addition & 0 deletions playground/assets/index.html
Expand Up @@ -3,6 +3,7 @@
<head>
<meta charset="UTF-8" />
<link class="ico" rel="icon" type="image/svg+xml" href="favicon.ico" />
<link rel="manifest" href="manifest.json" />
</head>

<link class="data-href" rel="icon" href="data:," />
Expand Down
1 change: 1 addition & 0 deletions playground/assets/manifest.json
@@ -0,0 +1 @@
{}

0 comments on commit 8ad81b4

Please sign in to comment.