fix: css insert order #9278

poyoho · 2022-07-21T14:09:12Z

Description

IIUC, the test case provided by #6044 will not get the expected results originally. But this test case exposes another problem.

When using chunk CSS, the insertion order of CSS in dev and build modes is different. Currently, there is no clear specification for CSS insertion order. So I chose the CSS sequence of build mode with higher performance as the benchmark to correct the inconsistent CSS insertion sequence.

In prod mode, Vite will concurrent request dependency for async chunk. It good point to optimize loading. https://vitejs.dev/guide/features.html#async-chunk-loading-optimization

Entry ---> (A + C)

But in dev load chunk will one by one.

Entry ---> A ---> C

For js it ok. But for css it will break the css insert order and make element node got an unexpected style.

To kept this optimization, we should keep dev in the same CSS insertion order as prod.

ensure dynamic import chunk entry point.

when meet a dynamic import will call preload helper to load all the deps in prod. It will insert <link rel="stylesheet" src="css url"> by BFS order.

Browser load the module is also BFS, but the dev module had the diff resolveId / load / transform hook handle timing, it will make the css order confusion.

In this PR, collect the loading order by browser.

mark dynamic import as entry point on importAnalysis plugin.
ensure the entry point weight on browser. (rewrite the import method to record the dynamic import order in browser and ~~inject the url query (http query params will break the browser cache)~~ send the weight state by websocket to shared the entry point weight to vite dev serve module graph)

via 1 collect state, we can ensure the file importer by what entry point in entry point moduleGraph.updateModuleInfo time point.

In this PR, add entryPoint field on ModuleNode to record the entry point for each module node.

insert the style node as the same as the collect loading weight.

via entry point weight we can ensure the style node weight as same as build mode.

After this PR, kept the build load async module order and mock the serve mode style order as same as build mode.

Additional context

TODO

hmr test
conditional triggered dynamic import
performance report

What is the purpose of this pull request?

Bug fix
New Feature
Documentation update
Other

Before submitting the PR, please make sure you do the following

Read the Contributing Guidelines.
Read the Pull Request Guidelines and follow the Commit Convention.
Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
Ideally, include relevant tests that fail without this PR but pass with it.

packages/vite/src/client/client.ts

packages/vite/src/node/server/moduleGraph.ts

jasikpark · 2022-07-21T14:31:50Z

packages/vite/src/node/server/moduleGraph.ts

@@ -164,6 +183,30 @@ export class ModuleGraph {
        }
      }
    })
+    // ensure the module entry point
+    mod.importers.forEach((importerMod) => {
+      if (importerMod.entryPoint === 'self') {


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"

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.

poyoho · 2022-07-21T14:33:38Z

Oh when I write the PR context, I find miss some context. Make it draft and do it later 🤦

IanVS

Thanks so much for working on this! I tested it out on my own project, and unfortunately I'm still seeing style problems. But the way that vite is bundling up the files is making it really hard to figure out why, or what exactly is happening. I'll keep trying to investigate, but it seems like normal .css files are being associated with .js files from a component library I'm using, though the two are not connected to each other other than I normally import both files into the same components. I'll report back if I can figure out more, or make a test case for it.

IanVS · 2022-07-25T19:57:42Z

playground/css/async-modules-and-css/blue.js

+import styles from './blue.module.css'
+
+const div = document.createElement('div')
+div.className = `base ${styles.blue} async-modules-and-css-blue`


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

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.

IanVS · 2022-07-25T20:05:04Z

playground/css/async-modules-and-css/black.js

+import './hotpink.css'
+
+const div = document.createElement('div')
+div.className = `base ${styles.black} async-modules-and-css-black`


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.

IanVS · 2022-07-25T22:12:33Z

I was able to reproduce the issue that I think I'm having, by importing a shared function into both blue and black.

IanVS@8c8bc43

In the case of both blue and black, I would expect their colors to be correct, since each one imports the css module after the global hotpink styles. But that doesn't happen, and one turns pink.

ghost · 2022-10-17T21:19:11Z

Is this PR still being worked on? It seems development has stopped on this.

atzcl · 2022-10-18T01:20:22Z

Is this PR still being worked on? It seems development has stopped on this.

It seems that the problem has been fixed, please see #9949

IanVS · 2022-10-18T01:22:29Z

@atzcl unfortunately not. That was one piece of it, but there are still problems which the PR you referenced did not address.

maicWorkGithub · 2023-03-29T05:36:04Z

As of today, more than 160 days have passed since the latest update. Is there any progress? Any plans to merge and release? We're looking forward to fixing this issue, we are blocked by this bug. thank you for your work!

maicWorkGithub · 2023-03-29T06:50:40Z

@poyoho Why this pr closed? Has this bug fixed or no need to fix? We blocked by #3924

poyoho · 2023-03-29T06:56:59Z

Because I feel like this pr can't solve the problem 🤦

patak-dev · 2023-03-29T07:38:43Z

@maicWorkGithub there are some new comments on #3924 (comment) that are hinting at this issue not being solvable. I suggest you look for a workaround, like the proposal there of using CSS layers. I don't think this issue is going to be solved any time soon and we may need to close it as expected.

maicWorkGithub · 2023-03-29T08:47:57Z

@poyoho Thanks for your work!
@patak-dev CSS layers too new, it's compatibility not suitable. postcss plugin of CSS layers seems a better way, I will suggest this in our dev group. Thanks.

poyoho added 7 commits June 23, 2022 23:57

test: css order

c1baca8

chore: rebase

24f4a32

feat: entry point for module graph

1d9f871

feat: inject style node

c806fac

chore: rebase

4033682

fix: inject order

cefe844

chore: update

5161e9a

poyoho added feat: css bug labels Jul 21, 2022

jasikpark reviewed Jul 21, 2022

View reviewed changes

packages/vite/src/client/client.ts Outdated Show resolved Hide resolved

jasikpark reviewed Jul 21, 2022

View reviewed changes

packages/vite/src/node/server/moduleGraph.ts Outdated Show resolved Hide resolved

jasikpark reviewed Jul 21, 2022

View reviewed changes

packages/vite/src/node/server/moduleGraph.ts Outdated Show resolved Hide resolved

jasikpark reviewed Jul 21, 2022

View reviewed changes

poyoho marked this pull request as draft July 21, 2022 14:33

poyoho added 8 commits July 22, 2022 13:56

fix: make entry point weight as same as browser module request order

a11ed72

test: nextTick import moudle should had the bigger weight

51ba6fc

chore: rebase

b926a1e

chore: css order

8174daf

chore: ensure module weight by browser

7eaa4be

feat: import.meta.glob css test

a70fcba

fix: ssr

ffe7269

fix: url origin request

1cb38fd

poyoho marked this pull request as ready for review July 23, 2022 17:49

poyoho marked this pull request as draft July 23, 2022 18:06

poyoho added 3 commits July 24, 2022 02:09

chore: update

065361c

chore: send weight by ws instead of request query

8291b9a

chore: update

817d0ed

poyoho marked this pull request as ready for review July 24, 2022 06:04

chore: typo

e0402b8

IanVS reviewed Jul 25, 2022

View reviewed changes

poyoho added 8 commits August 6, 2022 20:50

test: new test

65dc641

chore: update

5375f05

feat: module depth

8b48b7b

fix: depth calc error

782a9c4

feat: withRetry

ce828c4

chore: update

66a2109

chore: update

45ef18b

test: mulit depth test

8f4491d

IanVS mentioned this pull request Aug 11, 2022

test: Add failing tests for async css modules #6044

Closed

9 tasks

chenjiahan mentioned this pull request Aug 27, 2022

[Bug Report] vant3.6.0，tabbar组件bug youzan/vant#10953

Closed

patak-dev mentioned this pull request Sep 3, 2022

fix: css order problem in async chunk #9949

Merged

9 tasks

ghost mentioned this pull request Oct 17, 2022

Vite injects css assets in wrong order with dynamic import and css modules. #3924

Open

poyoho closed this Mar 29, 2023

jamesmcnamara mentioned this pull request Jun 25, 2024

Use vite for web builds sourcegraph/sourcegraph-public-snapshot#59219

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: css insert order #9278

fix: css insert order #9278

poyoho commented Jul 21, 2022 •

edited

Loading

jasikpark Jul 21, 2022

poyoho Jul 24, 2022

poyoho commented Jul 21, 2022 •

edited

Loading

IanVS left a comment

IanVS Jul 25, 2022

poyoho Aug 7, 2022 •

edited

Loading

IanVS Jul 25, 2022

IanVS commented Jul 25, 2022

ghost commented Oct 17, 2022

atzcl commented Oct 18, 2022 •

edited

Loading

IanVS commented Oct 18, 2022

maicWorkGithub commented Mar 29, 2023 •

edited

Loading

maicWorkGithub commented Mar 29, 2023

poyoho commented Mar 29, 2023

patak-dev commented Mar 29, 2023

maicWorkGithub commented Mar 29, 2023

fix: css insert order #9278

fix: css insert order #9278

Conversation

poyoho commented Jul 21, 2022 • edited Loading

Description

Additional context

What is the purpose of this pull request?

Before submitting the PR, please make sure you do the following

jasikpark Jul 21, 2022

Choose a reason for hiding this comment

poyoho Jul 24, 2022

Choose a reason for hiding this comment

poyoho commented Jul 21, 2022 • edited Loading

IanVS left a comment

Choose a reason for hiding this comment

IanVS Jul 25, 2022

Choose a reason for hiding this comment

poyoho Aug 7, 2022 • edited Loading

Choose a reason for hiding this comment

IanVS Jul 25, 2022

Choose a reason for hiding this comment

IanVS commented Jul 25, 2022

ghost commented Oct 17, 2022

atzcl commented Oct 18, 2022 • edited Loading

IanVS commented Oct 18, 2022

maicWorkGithub commented Mar 29, 2023 • edited Loading

maicWorkGithub commented Mar 29, 2023

poyoho commented Mar 29, 2023

patak-dev commented Mar 29, 2023

maicWorkGithub commented Mar 29, 2023

poyoho commented Jul 21, 2022 •

edited

Loading

poyoho commented Jul 21, 2022 •

edited

Loading

poyoho Aug 7, 2022 •

edited

Loading

atzcl commented Oct 18, 2022 •

edited

Loading

maicWorkGithub commented Mar 29, 2023 •

edited

Loading