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

Import statements are in reverse order #53

Closed
JannikGM opened this issue Jun 29, 2023 · 2 comments · Fixed by #54
Closed

Import statements are in reverse order #53

JannikGM opened this issue Jun 29, 2023 · 2 comments · Fixed by #54

Comments

@JannikGM
Copy link
Contributor

We have some CSS which is affected by vitejs/vite#3924

We should add the imports in order of appearance.
Imports are correctly iterated in map insertion order.
However, they are added in reverse order due to ast.program.body.unshift.

There might be additional issues when mixing require and import, but we can probably ignore them for now.

@JannikGM
Copy link
Contributor Author

JannikGM commented Jul 20, 2023

#54 has massively improved the CSS for us (where order is important), but now we still have a bunch of styling bugs (which went unnoticed during development and testing of #54).
I'll try to debug this further in the near future.

Among them is a bug with "@material-ui/core": "^4.11.4",:

Working Result (how it should look like)

Screenshot 2023-07-20 at 16 16 39

Broken Result (what it currently looks like)

Screenshot 2023-07-20 at 16 16 27

Broken CSS

Screenshot 2023-07-20 at 16 16 17

Broken HTML

Screenshot 2023-07-20 at 16 08 38

@JannikGM
Copy link
Contributor Author

We solved the problem above with StylesProvider from mui: mui/material-ui#11843 (comment).
We already had a hack using JssProvider from react-jss, but that didn't work in Vite (not sure what it did for webpack).

I'm not sure why there's a difference between webpack and vite for this or why the JssProvider didn't work, but that's not important for us, now that it works.

So I believe everything works as intended now 👍

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 a pull request may close this issue.

1 participant