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.

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

fix: css insert order #9278

Closed
wants to merge 27 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 50 additions & 2 deletions packages/vite/src/client/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -324,8 +324,34 @@ const sheetsMap = new Map<
HTMLStyleElement | CSSStyleSheet | undefined
>()

export function updateStyle(id: string, content: string): void {
interface CSSAnchor {
start: HTMLStyleElement
end: HTMLStyleElement
weight: number
}

const anchorMap = new Map<string, CSSAnchor>()

function binarySearchCSSInjectPosition(weight: number): HTMLElement | null {
const list = Array.from(anchorMap.values()).sort(
(a, b) => a.weight - b.weight
)
for (const anchor of list) {
if (weight < anchor.weight) {
return anchor.end
}
}
return null
}

export function updateStyle(
id: string,
content: string,
moduleEntry: string,
entryWeight: number
): void {
let style = sheetsMap.get(id)
// TODO inject the truth oreder for stylesheet
if (supportsConstructedSheet && !content.includes('@import')) {
if (style && !(style instanceof CSSStyleSheet)) {
removeStyle(id)
Expand All @@ -352,7 +378,29 @@ export function updateStyle(id: string, content: string): void {
style = document.createElement('style')
style.setAttribute('type', 'text/css')
style.innerHTML = content
document.head.appendChild(style)
// for debugging
style.setAttribute('w', entryWeight.toString())
if (moduleEntry === 'main') {
document.head.appendChild(style)
} else {
const anchor = anchorMap.get(moduleEntry)
if (anchor) {
document.head.insertBefore(style, anchor.end.nextSibling)
anchor.end = style
} else {
const node = binarySearchCSSInjectPosition(entryWeight)
anchorMap.set(moduleEntry, {
start: style,
end: style,
weight: entryWeight
})
if (node) {
document.head.insertBefore(style, node)
} else {
document.head.appendChild(style)
}
}
}
} else {
style.innerHTML = content
}
Expand Down
19 changes: 18 additions & 1 deletion packages/vite/src/node/plugins/css.ts
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@ export function cssPlugin(config: ResolvedConfig): Plugin {
moduleGraph.updateModuleInfo(
thisModule,
depModules,
depModules,
null,
// The root CSS proxy module is self-accepting and should not
// have an explicit accept list
Expand Down Expand Up @@ -298,6 +299,7 @@ export function cssPostPlugin(config: ResolvedConfig): Plugin {
// styles initialization in buildStart causes a styling loss in watch
const styles: Map<string, string> = new Map<string, string>()
let pureCssChunks: Set<string>
let serve: ViteDevServer

// when there are multiple rollup outputs and extracting CSS, only emit once,
// since output formats have no effect on the generated CSS.
Expand Down Expand Up @@ -336,6 +338,10 @@ export function cssPostPlugin(config: ResolvedConfig): Plugin {
hasEmitted = false
},

configureServer(_server) {
serve = _server
},

async transform(css, id, options) {
if (
!isCSSRequest(id) ||
Expand Down Expand Up @@ -379,13 +385,24 @@ export function cssPostPlugin(config: ResolvedConfig): Plugin {

const cssContent = await getContentWithSourcemap(css)
const devBase = config.base
const moduleNode = serve.moduleGraph.getModuleById(id)!
const entryPoint =
typeof moduleNode.entryPoint === 'string'
? moduleNode.entryPoint
: moduleNode.entryPoint!.id
const entryPointWeight =
typeof moduleNode.entryPoint === 'string'
? 0
: moduleNode.entryPoint!.weight
return [
`import { updateStyle as __vite__updateStyle, removeStyle as __vite__removeStyle } from ${JSON.stringify(
path.posix.join(devBase, CLIENT_PUBLIC_PATH)
)}`,
`const __vite__id = ${JSON.stringify(id)}`,
`const __vite__css = ${JSON.stringify(cssContent)}`,
`__vite__updateStyle(__vite__id, __vite__css)`,
`const __vite__entry = ${JSON.stringify(entryPoint)}`,
`const __vite__entry_weight = ${JSON.stringify(entryPointWeight)}`,
`__vite__updateStyle(__vite__id, __vite__css, __vite__entry, __vite__entry_weight)`,
// css modules exports change on edit so it can't self accept
`${
modulesCode ||
Expand Down
1 change: 1 addition & 0 deletions packages/vite/src/node/plugins/importAnalysis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
const prunedImports = await moduleGraph.updateModuleInfo(
importerModule,
importedUrls,
new Set(Array.from(staticImportedUrls).map((item) => item.url)),
importedBindings,
normalizedAcceptedUrls,
isPartiallySelfAccepting ? acceptedExports : null,
Expand Down
43 changes: 43 additions & 0 deletions packages/vite/src/node/server/moduleGraph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import {
import { FS_PREFIX } from '../constants'
import type { TransformResult } from './transformRequest'

let weight = 0

export class ModuleNode {
/**
* Public served url path, starts with /
Expand All @@ -35,6 +37,18 @@ export class ModuleNode {
ssrError: Error | null = null
lastHMRTimestamp = 0
lastInvalidationTimestamp = 0
/**
* entryPoint of the module
* - null - no init
* - self - it's entry pointer (the ModuleNode what import by dynamic import)
* - main - it's import by main entry point
* - ModuleNode - it's import by dynamic import and the entry point is this field
*/
entryPoint: ModuleNode | 'self' | 'main' | null = null
/**
* lower weight is better
*/
weight: number = 0

/**
* @param setIsSelfAccepting - set `false` to set `isSelfAccepting` later. e.g. #7870
Expand Down Expand Up @@ -135,6 +149,7 @@ export class ModuleGraph {
async updateModuleInfo(
mod: ModuleNode,
importedModules: Set<string | ModuleNode>,
staticImportUrls: Set<string | ModuleNode>,
importedBindings: Map<string, Set<string>> | null,
acceptedModules: Set<string | ModuleNode>,
acceptedExports: Set<string> | null,
Expand All @@ -153,6 +168,10 @@ export class ModuleGraph {
: imported
dep.importers.add(mod)
nextImports.add(dep)
if (!staticImportUrls.has(imported)) {
dep.entryPoint = 'self'
dep.weight = ++weight
}
}
// remove the importer from deps that were imported but no longer are.
prevImports.forEach((dep) => {
Expand All @@ -164,6 +183,30 @@ export class ModuleGraph {
}
}
})
// ensure the module entry point
mod.importers.forEach((importerMod) => {
if (importerMod.entryPoint === 'self') {

Choose a reason for hiding this comment

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

a question - how do circular deps affect something like this? I guess that's just a general ESM problem of which the answer is "circular deps are illegal"

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for review. I think this is no circular deps affect problem. Because mod just find the parent node (mod.importers) which is first to use the mod.

if (mod.entryPoint) {
mod.entryPoint =
importerMod.weight < mod.weight ? importerMod : mod.entryPoint
} else {
mod.entryPoint = importerMod
}
} else if (importerMod.entryPoint instanceof ModuleNode) {
if (mod.entryPoint) {
mod.entryPoint =
importerMod.entryPoint.weight < mod.weight
? importerMod.entryPoint
: mod.entryPoint
} else {
mod.entryPoint = importerMod.entryPoint
}
}
})
if (mod.entryPoint === null) {
mod.entryPoint = 'main'
mod.weight = 0
}
// update accepted hmr deps
const deps = (mod.acceptedHmrDeps = new Set())
for (const accepted of acceptedModules) {
Expand Down
29 changes: 29 additions & 0 deletions playground/css/__tests__/css.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,35 @@ test('aliased css has content', async () => {
expect(await getColor('.aliased-module')).toBe('blue')
})

test('async css modules', async () => {
const green = await page.$('.async-modules-green')
const blue2 = await page.$('.async-modules-blue2')
const red = await page.$('.async-modules-red')
const blue = await page.$('.async-modules-blue')

const _black = await page.$('.async-modules-and-css-black')
const _blue = await page.$('.async-modules-and-css-blue')

expect(await getColor(green)).toBe('green')
expect(await getColor(blue2)).toBe('blue')

expect(await getColor(_black)).toBe('black')
expect(await getColor(_blue)).toBe('blue')

// because that loaded blue > red first
// and can't change the style order
expect(await getColor(blue)).toBe('black')
expect(await getColor(red)).toBe('black')
})

test('async css modules with normal css', async () => {
const black = await page.$('.async-modules-and-css-black')
const blue = await page.$('.async-modules-and-css-blue')

expect(await getColor(black)).toBe('black')
expect(await getColor(blue)).toBe('blue')
})

test.runIf(isBuild)('warning can be suppressed by esbuild.logOverride', () => {
serverLogs.forEach((log) => {
// no warning from esbuild css minifier
Expand Down
9 changes: 9 additions & 0 deletions playground/css/async-modules-and-css/black.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import styles from './black.module.css'
import './hotpink.css'

const div = document.createElement('div')
div.className = `base ${styles.black} async-modules-and-css-black`
Copy link
Contributor

Choose a reason for hiding this comment

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

hotpink seems unused here. Maybe it should be added to the class list, and the assertion should check that the text is pink, since it is loaded last.

document.body.appendChild(div)
div.textContent = `async css modules and normal css (black) ${
getComputedStyle(div).color
}`
3 changes: 3 additions & 0 deletions playground/css/async-modules-and-css/black.module.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.black {
color: black;
}
9 changes: 9 additions & 0 deletions playground/css/async-modules-and-css/blue.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import './hotpink.css'
import styles from './blue.module.css'

const div = document.createElement('div')
div.className = `base ${styles.blue} async-modules-and-css-blue`
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this class should also contain hotpink, to verify that the blue style "wins" because it is imported second.

Copy link
Member Author

@poyoho poyoho Aug 7, 2022

Choose a reason for hiding this comment

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

yes. I think the css order should as same as build load module order

if the module is a chunk will insert into head later.

document.body.appendChild(div)
div.textContent = `async css modules and normal css (blue) ${
getComputedStyle(div).color
}`
3 changes: 3 additions & 0 deletions playground/css/async-modules-and-css/blue.module.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.blue {
color: blue;
}
4 changes: 4 additions & 0 deletions playground/css/async-modules-and-css/hotpink.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
.hotpink {
font-size: 1rem;
color: hotpink;
}
10 changes: 10 additions & 0 deletions playground/css/async-modules/base.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import styles from './black.module.css'

export function baseAsync(className, color) {
const div = document.createElement('div')
div.className = `${styles.black} ${className} async-modules-${color}`
document.body.appendChild(div)
div.textContent = `async css modules (${color}) ${
getComputedStyle(div).color
}`
}
10 changes: 10 additions & 0 deletions playground/css/async-modules/base2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import styles from './red.module.css'

export function baseAsync(className, color) {
const div = document.createElement('div')
div.className = `${styles.red} ${className} async-modules-${color}`
document.body.appendChild(div)
div.textContent = `async css modules2 (${color}) ${
getComputedStyle(div).color
}`
}
3 changes: 3 additions & 0 deletions playground/css/async-modules/black.module.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.black {
color: black;
}
4 changes: 4 additions & 0 deletions playground/css/async-modules/blue.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import styles from './blue.module.css'
import { baseAsync } from './base'

baseAsync(styles.blue, 'blue')
3 changes: 3 additions & 0 deletions playground/css/async-modules/blue.module.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.blue {
color: blue;
}
6 changes: 6 additions & 0 deletions playground/css/async-modules/green.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { baseAsync } from './base2'
import green from './green.module.css'
import blue from './blue.module.css'

baseAsync(green.green, 'green')
baseAsync(blue.blue, 'blue2')
3 changes: 3 additions & 0 deletions playground/css/async-modules/green.module.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.green {
color: green;
}
4 changes: 4 additions & 0 deletions playground/css/async-modules/red.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { baseAsync } from './base'
import styles from './red.module.css'

baseAsync(styles.red, 'red')
3 changes: 3 additions & 0 deletions playground/css/async-modules/red.module.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.red {
color: red;
}
9 changes: 9 additions & 0 deletions playground/css/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,3 +106,12 @@ document
.classList.add(aliasModule.aliasedModule)

import './unsupported.css'

// async css modules
import('./async-modules/blue')
import('./async-modules/red')
import('./async-modules/green')

// async css module with normal css
import('./async-modules-and-css/black')
import('./async-modules-and-css/blue')