Skip to content

Commit

Permalink
fix(build): mixed external and transpiled srcset (#14888)
Browse files Browse the repository at this point in the history
  • Loading branch information
@patak-dev
patak-dev authored Nov 7, 2023
1 parent bd4d29f commit b5653d3
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 76 deletions.
177 changes: 103 additions & 74 deletions packages/vite/src/node/plugins/html.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,11 +294,10 @@ export function buildHtmlPlugin(config: ResolvedConfig): Plugin {
preHooks.push(htmlEnvHook(config))
postHooks.push(postImportMapHook())
const processedHtml = new Map<string, string>()

const isExcludedUrl = (url: string) =>
url[0] === '#' ||
isExternalUrl(url) ||
isDataUrl(url) ||
checkPublicFile(url, config)
url[0] === '#' || isExternalUrl(url) || isDataUrl(url)

// Same reason with `htmlInlineProxyPlugin`
isAsyncScriptMap.set(config, new Map())

Expand Down Expand Up @@ -367,10 +366,6 @@ export function buildHtmlPlugin(config: ResolvedConfig): Plugin {

let js = ''
const s = new MagicString(html)
const assetUrls: {
attr: Token.Attribute
sourceCodeLocation: Token.Location
}[] = []
const scriptUrls: ScriptAssetsUrl[] = []
const styleUrls: ScriptAssetsUrl[] = []
let inlineModuleIndex = -1
Expand All @@ -379,6 +374,31 @@ export function buildHtmlPlugin(config: ResolvedConfig): Plugin {
let someScriptsAreAsync = false
let someScriptsAreDefer = false

const assetUrlsPromises: Promise<void>[] = []

// for each encountered asset url, rewrite original html so that it
// references the post-build location, ignoring empty attributes and
// attributes that directly reference named output.
const namedOutput = Object.keys(
config?.build?.rollupOptions?.input || {},
)
const processAssetUrl = async (url: string) => {
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)
} catch (e) {
if (e.code !== 'ENOENT') {
throw e
}
}
}
return url
}

await traverseHtml(html, id, (node) => {
if (!nodeIsElement(node)) {
return
Expand All @@ -404,7 +424,7 @@ export function buildHtmlPlugin(config: ResolvedConfig): Plugin {

if (isModule) {
inlineModuleIndex++
if (url && !isExcludedUrl(url)) {
if (url && !isExcludedUrl(url) && !isPublicFile) {
// <script type="module" src="..."/>
// add it as an import
js += `\nimport ${JSON.stringify(url)}`
Expand Down Expand Up @@ -447,41 +467,71 @@ export function buildHtmlPlugin(config: ResolvedConfig): Plugin {
for (const p of node.attrs) {
const attrKey = getAttrKey(p)
if (p.value && assetAttrs.includes(attrKey)) {
const attrSourceCodeLocation =
node.sourceCodeLocation!.attrs![attrKey]
// assetsUrl may be encodeURI
const url = decodeURI(p.value)
if (!isExcludedUrl(url)) {
if (
node.nodeName === 'link' &&
isCSSRequest(url) &&
// should not be converted if following attributes are present (#6748)
!node.attrs.some(
(p) =>
p.prefix === undefined &&
(p.name === 'media' || p.name === 'disabled'),
if (attrKey === 'srcset') {
assetUrlsPromises.push(
(async () => {
const processedUrl = await processSrcSet(
p.value,
async ({ url }) => {
const decodedUrl = decodeURI(url)
if (!isExcludedUrl(decodedUrl)) {
const result = await processAssetUrl(url)
return result !== decodedUrl ? result : url
}
return url
},
)
if (processedUrl !== p.value) {
overwriteAttrValue(
s,
getAttrSourceCodeLocation(node, attrKey),
processedUrl,
)
}
})(),
)
} else {
const url = decodeURI(p.value)
if (checkPublicFile(url, config)) {
overwriteAttrValue(
s,
getAttrSourceCodeLocation(node, attrKey),
toOutputPublicFilePath(url),
)
) {
// CSS references, convert to import
const importExpression = `\nimport ${JSON.stringify(url)}`
styleUrls.push({
url,
start: nodeStartWithLeadingWhitespace(node),
end: node.sourceCodeLocation!.endOffset,
})
js += importExpression
} else {
assetUrls.push({
attr: p,
sourceCodeLocation: attrSourceCodeLocation,
})
} else if (!isExcludedUrl(url)) {
if (
node.nodeName === 'link' &&
isCSSRequest(url) &&
// should not be converted if following attributes are present (#6748)
!node.attrs.some(
(p) =>
p.prefix === undefined &&
(p.name === 'media' || p.name === 'disabled'),
)
) {
// CSS references, convert to import
const importExpression = `\nimport ${JSON.stringify(url)}`
styleUrls.push({
url,
start: nodeStartWithLeadingWhitespace(node),
end: node.sourceCodeLocation!.endOffset,
})
js += importExpression
} else {
assetUrlsPromises.push(
(async () => {
const processedUrl = await processAssetUrl(url)
if (processedUrl !== url) {
overwriteAttrValue(
s,
getAttrSourceCodeLocation(node, attrKey),
processedUrl,
)
}
})(),
)
}
}
} else if (checkPublicFile(url, config)) {
overwriteAttrValue(
s,
attrSourceCodeLocation,
toOutputPublicFilePath(url),
)
}
}
}
Expand Down Expand Up @@ -543,42 +593,14 @@ export function buildHtmlPlugin(config: ResolvedConfig): Plugin {
)
}

// for each encountered asset url, rewrite original html so that it
// references the post-build location, ignoring empty attributes and
// attributes that directly reference named output.
const namedOutput = Object.keys(
config?.build?.rollupOptions?.input || {},
)
for (const { attr, sourceCodeLocation } of assetUrls) {
// assetsUrl may be encodeURI
const content = decodeURI(attr.value)
if (
content !== '' && // Empty attribute
!namedOutput.includes(content) && // Direct reference to named output
!namedOutput.includes(removeLeadingSlash(content)) // Allow for absolute references as named output can't be an absolute path
) {
try {
const url =
attr.prefix === undefined && attr.name === 'srcset'
? await processSrcSet(content, ({ url }) =>
urlToBuiltUrl(url, id, config, this),
)
: await urlToBuiltUrl(content, id, config, this)
await Promise.all(assetUrlsPromises)

overwriteAttrValue(s, sourceCodeLocation, url)
} catch (e) {
if (e.code !== 'ENOENT') {
throw e
}
}
}
}
// emit <script>import("./aaa")</script> asset
for (const { start, end, url } of scriptUrls) {
if (!isExcludedUrl(url)) {
s.update(start, end, await urlToBuiltUrl(url, id, config, this))
} else if (checkPublicFile(url, config)) {
if (checkPublicFile(url, config)) {
s.update(start, end, toOutputPublicFilePath(url))
} else if (!isExcludedUrl(url)) {
s.update(start, end, await urlToBuiltUrl(url, id, config, this))
}
}

Expand Down Expand Up @@ -1331,3 +1353,10 @@ function incrementIndent(indent: string = '') {
export function getAttrKey(attr: Token.Attribute): string {
return attr.prefix === undefined ? attr.name : `${attr.prefix}:${attr.name}`
}

function getAttrSourceCodeLocation(
node: DefaultTreeAdapterMap['element'],
attrKey: string,
) {
return node.sourceCodeLocation!.attrs![attrKey]
}
3 changes: 1 addition & 2 deletions playground/assets/__tests__/assets.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,7 @@ describe('image', () => {
})
})

// TODO: fix build
test.runIf(!isBuild)('srcset (mixed)', async () => {
test('srcset (mixed)', async () => {
const img = await page.$('.img-src-set-mixed')
const srcset = await img.getAttribute('srcset')
const srcs = srcset.split(', ')
Expand Down

0 comments on commit b5653d3

Please sign in to comment.