vite icon indicating copy to clipboard operation
vite copied to clipboard

fix: css insert order

Open poyoho opened this issue 1 year ago • 2 comments

Description

fix #3924 relate #6301 close: #6044

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.

  1. 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)
  1. 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.

  1. 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?

  • [x] Bug fix
  • [ ] New Feature
  • [ ] Documentation update
  • [ ] Other

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

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

poyoho avatar Jul 21 '22 14:07 poyoho

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

poyoho avatar Jul 21 '22 14:07 poyoho

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

https://github.com/IanVS/vite/commit/8c8bc4306083427a9946274ae1fb3cadf0d3cf3f

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.

IanVS avatar Jul 25 '22 22:07 IanVS

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

ghost avatar Oct 17 '22 21:10 ghost

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

It seems that the problem has been fixed, please see https://github.com/vitejs/vite/pull/9949

atzcl avatar Oct 18 '22 01:10 atzcl

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

IanVS avatar Oct 18 '22 01:10 IanVS

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 avatar Mar 29 '23 05:03 maicWorkGithub

@poyoho Why this pr closed? Has this bug fixed or no need to fix? We blocked by https://github.com/vitejs/vite/issues/3924

maicWorkGithub avatar Mar 29 '23 06:03 maicWorkGithub

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

poyoho avatar Mar 29 '23 06:03 poyoho

@maicWorkGithub there are some new comments on https://github.com/vitejs/vite/issues/3924#issuecomment-1452186508 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.

patak-dev avatar Mar 29 '23 07:03 patak-dev

@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.

maicWorkGithub avatar Mar 29 '23 08:03 maicWorkGithub