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

all: stop importing CSS files from library code #837

Closed
wants to merge 1 commit into from

Conversation

emersion
Copy link
Member

We were importing CSS files directly from library code. This causes issues for downstream users, because they are forced to configure their build system to support CSS imports (these aren't standard JS, e.g. webpack doesn't support this by default). Moreover, this makes it impossible to use the library without a build system (e.g. importing it directly from pure HTML/JS files).

Make users responsible for including the CSS one way or another. Our stories do this using CSS imports already, but other downstream consumers might prefer to use a <link> or similar.

@emersion emersion requested review from Yohh and Math-R January 20, 2025 16:42
@emersion emersion requested a review from a team as a code owner January 20, 2025 16:42
@emersion emersion force-pushed the emr/no-css-imports branch 2 times, most recently from 83fc61b to b04c0aa Compare January 20, 2025 16:52
@emersion emersion self-assigned this Jan 20, 2025
We were importing CSS files directly from library code. This causes
issues for downstream users, because they are forced to configure
their build system to support CSS imports (e.g. webpack doesn't
support this by default). Moreover, this makes it impossible to use
the library without a build system (e.g. importing it directly from
pure HTML/JS files).

Make users responsible for including the CSS one way or another.
Our stories do this using CSS imports already, but other downstream
consumers might prefer to use a <link> or similar.

Signed-off-by: Simon Ser <[email protected]>
@emersion
Copy link
Member Author

Hm so… Turns out this is trickier than anticipated.

dist/theme.css gets generated by rollup-plugin-postcss, as a side effect of importing a CSS file from JS. Dropping the imports makes it so rollup-plugin-postcss no longer collects any CSS file as input.

However, the issue isn't the ./styles/main.css import that we have: that one gets stripped away from the final JS. The issue is the @osrd-project/ui-core/dist/theme.css import which doesn't get stripped.

I've found a better solution to this problem: #852

@emersion emersion closed this Jan 22, 2025
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.

1 participant