Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: warn for unresolved css in html #7911

Merged
merged 7 commits into from May 4, 2022
Merged

fix: warn for unresolved css in html #7911

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
219dda2
fix: warn for unresolved css in html
poyoho Apr 26, 2022
77bcec0
feat: promise all
poyoho Apr 26, 2022
1a84a44
fix: remove node
poyoho Apr 26, 2022
2560d4b
chore: comment
poyoho Apr 27, 2022
e1b92fb
feat: kept import order
poyoho Apr 27, 2022
829ebca
feat: promise.all
poyoho Apr 29, 2022
9afaa75
fix: errors
poyoho May 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/playground/html/index.html
View file
Open in desktop
Expand Up @@ -5,3 +5,6 @@ <h1>Hello</h1>
<!-- comment two -->
<script type="module"></script>
<script type="module" src="/main.js"></script>
<link rel="icon" href="{{cdn_host}}/cdn/images/favicon.ico" />
<link rel="stylesheet" href="{{cdn_host}}/css.css" type="text/css" />
<script src="{{cdn_host}}/js.js"></script>
29 changes: 27 additions & 2 deletions packages/vite/src/node/plugins/html.ts
View file
Open in desktop
Expand Up @@ -246,6 +246,7 @@ export function buildHtmlPlugin(config: ResolvedConfig): Plugin {
const s = new MagicString(html)
const assetUrls: AttributeNode[] = []
const scriptUrls: ScriptAssetsUrl[] = []
const styleUrls: ScriptAssetsUrl[] = []
let inlineModuleIndex = -1

let everyScriptIsAsync = true
Expand Down Expand Up @@ -338,8 +339,13 @@ export function buildHtmlPlugin(config: ResolvedConfig): Plugin {
if (!isExcludedUrl(url)) {
if (node.tag === 'link' && isCSSRequest(url)) {
// CSS references, convert to import
js += `\nimport ${JSON.stringify(url)}`
shouldRemove = true
const importExpression = `\nimport ${JSON.stringify(url)}`
styleUrls.push({
url,
start: node.loc.start.offset,
end: node.loc.end.offset
})
Comment on lines +343 to +347
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still unsure about the order change in the JS that this is going to produce. IIUC, before if the user had first the CSS styles, these were the first imports in the JS, and now they will be imported at the end after all other JS tags. I wonder if we should mark the JS position by adding something like

  js += `${STYLE_IMPORT_MARKER_}${styleUrlId}`

And then we can replace it by \nimport ... or by `` depending on the result of the URL resolution.

js += importExpression
} else {
assetUrls.push(p)
}
Expand Down Expand Up @@ -469,6 +475,25 @@ export function buildHtmlPlugin(config: ResolvedConfig): Plugin {
}
}

// ignore <link rel="stylesheet"> if its url can't be resolved
const resolvedStyleUrls = await Promise.all(
styleUrls.map(async (styleUrl) => ({
...styleUrl,
resolved: await this.resolve(styleUrl.url, id)
}))
)
for (const { start, end, url, resolved } of resolvedStyleUrls) {
if (resolved == null) {
config.logger.warnOnce(
`\n${url} doesn't exist at build time, it will remain unchanged to be resolved at runtime`
)
const importExpression = `\nimport ${JSON.stringify(url)}`
js = js.replace(importExpression, '')
} else {
s.remove(start, end)
}
}

processedHtml.set(id, s.toString())

// inject module preload polyfill only when configured and needed
Expand Down