-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
fix: allow css to be written for systemjs output #5902
Conversation
c76c636
to
432515a
Compare
@patak-js is this wanted? |
Really appreciate your time and please consider merging this PR, I strongly believe it's not a matter of "wanted" or not. It is actually a missing feature as it should work as stated from the docs description:
If people don't want it, they can use the already available |
Sorry for keep pushing @Shinigami92 @patak-dev But we really need this merged, appreciate your time if you can facilitate the merge and release that would be ideal, otherwise, if you can respond with a reason for the rejection would also be appreciated and what we can do or change to make it fit with your envision. Many thanks. |
I think it's not a thing of approve or reject, but current capacity of vite team over the winter holidays. |
Hey @shir0u, sorry for the delay, we missed this one in our previous team meeting. I don't know why system wasn't included here, we'll discuss it in the next meeting. If this PR is blocking you, I recommend that you vendor or fork Vite until this gets resolved upstream. |
Thanks @patak-dev I've created a fork already on npm which is fine for now, but needs to rebase every now and then. Was wondering if you guys had your meeting yet? |
@shir0u we talked about the PR in our last team meeting, and we are good to go if we can test before that it doesn't break plugin-legacy. Would it be possible to add a test for dynamic CSS chuncks in |
Hi @patak-dev this current PR has passed all current tests, as for adding new tests, looks like it's beyond my expertise, would you or your team be able to give me a hand with this? Many thanks, much appreciated. |
2e18a9c
to
46f0a53
Compare
Hi @patak-dev I have attempted to add that dynamic CSS config, not sure where else to do it. Please let me know if I am missing something. Thanks. Very keen to get this merged as soon as possible. |
Hi @patak-dev have rebased and all tests seem to have passed. Please review at your earliest convenience. Many thanks. |
This PR broke Also does this PR work as intended? |
Looks like this PR works with |
This reverts commit 780b4f5.
Description
According to https://vitejs.dev/guide/features.html#css-code-splitting
However, when building to systemjs output, this feature seems to be ignored. This PR fixes #5901
Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).