-
Notifications
You must be signed in to change notification settings - Fork 144
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(preservemodules): temporarily remove preserveModules #601
Conversation
|
Thanks @maiieul My question now is - is this critical for the release of headless and fluffy, or is it just an optimization that could be done later? |
I think it's a bit critical yes. Fluffy should provide the Accordion components named as "Accordion", and right now we can't. Also if we want to appear as a serious alternative to other frameworks even for UI, we need to have a docs site that feels more performant (which is not the case right now because of the CLS issues and the big bundle with shikiji. Besides, the namespace issue will be problematic for any other component where we'd like to reuse the same names for the styled components as the headless components. For me it's having "preserveModules" that is an optimization that can be added later. It's also very easy to add back. For me the problem is that we don't know when this will be fixed in Qwik. Could be in 3 months but maybe 6 or even a year from now... |
P.S. I think we need to prioritize our DX (fast development & no bugs) before slightly better perf for the end user for now. Also I believe that the fact that we can't really use preserveModules with the current conditions should be pressing Misko or Manu to prioritize this before other issues. Helping library authors is vital to Qwik's ecosystem and I'm sure they understand that. |
LGTM |
If we are not merging this, then it also means we'll have to ask all our users at some point to change the name of their components - unless we can find a name that can stay, but I will always prefer "Accordion" to anything else. So if we don't merge this, what prefix/suffix should we use? So a few ideas: Accordion$ In my opinion Accordion$ or $Accordion look the cleanest. Maybe Usually things like "_Accordion" are for internal use, no? It feels awkward to me. $ is more in the spirit of Qwik if we decide to go down that path 😉 |
merged |
What is it?
Why is it needed?
For two reasons:
Until QwikDev/qwik#5473 gets solved in Qwik, I say we temporarily disable preserveModules. We will have less performance (it should be about 11kb more instead of 40 now that the repo is cleaned up) in favor of more stability.
Checklist:
pnpm changeset
and documented my changes