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

vite: run transform as early as possible #94

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wmertens
Copy link

This is conceptually correct because style9 has to have been converted at bundle time. This fixes Qwik bundling, for example.

@johanholmerin
Copy link
Owner

@nonzzz Do you want to weigh in on this?

This is conceptually correct because style9 has to have been converted at bundle time.
This fixes Qwik bundling, for example.
@nonzzz
Copy link
Contributor

nonzzz commented May 11, 2023

@johanholmerin ok.I will take a look

@wmertens
Copy link
Author

I must add that this is not quite the full solution for Qwik, because of the chunk processing and some weirdness around dev mode.

To make style9 work in Qwik, you need:

  1. this PR
  2. define your styles inside the scope of the component$(), or use export syntax and get only the classnames
  3. run in preview mode
  4. disable the renderChunk hook in vite.js

Reasons:

  1. so the transform runs before Qwik does export magic
  2. when building for prod, Qwik will still split up the source before style9 gets to transform, and so the babel transform won't see the styles() call and not generate the CSS
  3. in dev mode, the css file should be available as a separate file for a <link/> tag but that doesn't seem to work, so the CSS is not in the browser
  4. Qwik removes the CSS import calls and merges everything. the renderChunk puts style9 css in a separate file after this happens, and it also never gets imported

@nonzzz
Copy link
Contributor

nonzzz commented May 12, 2023

Sorry. I'm Qwik noob. I will take a look at the weekend.

@wmertens wmertens marked this pull request as draft May 12, 2023 07:06
@wmertens
Copy link
Author

Hmm - here's a blitz using the unchanged module: https://stackblitz.com/edit/qwik-style9?file=src%2Froutes%2Findex.tsx

This almost works out of the box because I have the definition inside component$ and it doesn't get split into a separate file by Qwik.

Dev mode works as well, after the first re-render of the component. The reason is that the name of the css import is wrong the first time, and on re-render it imports the css with the correct name.

So this means that the order: 'pre' is not strictly needed right now, so I set this PR to draft.

As for the renderChunks, it would be nice if it's optional so there's not two CSS files that are imported. You can see this happening by running npm run preview in the blitz, all CSS files are in one style tag and the style9 CSS is in another style tag.

(Qwik automatically inlines CSS that is less than 10KB)

@nonzzz
Copy link
Contributor

nonzzz commented May 13, 2023

@wmertens
Sorry for the wait.Why we need renderChunk. Because. If we don't using this. it can't prcoess the right css chunk. Because in build mode. The load hook only
be trigger once. So us virtual module can't get the full css info. So we need find the virtual module in renderChunk stage and rewrite them
. According the source code. you can see the follow code chunk.viteMetadata.importedCss.add. I think i can make a optimization for this.
Don't worry. I'll make a RP after verifying if it work.

@wmertens
Copy link
Author

@nonzzz I don't understand, every time style9 CSS is encountered the virtual .css file is invalidated, so it will contain eventually all the css, no? I don't see extra build steps in renderChunks?

I'm still trying to figure out how the qwik vite plugin works in build mode. The files are split up before the style9 transform runs, and ideally style9 runs before qwik parses the files.

@nonzzz
Copy link
Contributor

nonzzz commented May 14, 2023

@wmertens renderChunk only work with build mode. If remove this logic. vite can't process the right css chunk. Because in build mode. resolve and load hook only be called once.( Becasue we return a virtual module. It only record the first load state.)

It looks like we create many javascript file and define style9.create and using it. Let called it as a.js and b.js.

When rollup run. The transform hook handle a.js and b.js (do not care about processing order)
And we return a virtual module. And the load hook is excute. then we process b.js. It also return
a virtual module. rollup will compare the module. If same skip. So the renderChunk hook can't get the latest css.

CSS code splitting is a function provided by vite. Currently. we remove the virutal module in renderChunk hook. and rewrite the latest css code and emit then bind it to vite. I made an optimization for this see #95 ( you can try this and test again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants