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

Let Expo/Webpack handle CSS assets #3942

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

mary-ext
Copy link
Contributor

@mary-ext mary-ext commented May 10, 2024

Stacked on #3929, see #3929 (comment)

Manually sharing CSS code between web/index.html (dev builds) and bskyweb/templates/base.html (production) is brittle, let's have it handled as a built asset instead.

This also means hot-reloading on development, well, hopefully it should.

Small savings gained from minification (reduction of ~2kb uncompressed)

Comment on lines -256 to -257
</style>
</style>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never even noticed the double closing tag until now 😅

@@ -1,5 +1,6 @@
import 'lib/sentry' // must be near top
import 'view/icons'
import './style.css'
Copy link
Contributor Author

@mary-ext mary-ext May 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This import order isn't right, ideally this should be imported very last but the ESLint sort import plugin doesn't seem to like that, not sure what I'd do to make this right. Maybe it's okay right now though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can edit the sort plugin's order to put css files at the end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's what I was unsure about, because according to the config it should be putting relative imports last?

social-app/.eslintrc.js

Lines 52 to 64 in 195c9f1

// Relative imports.
// Ideally, anything that starts with a dot or #
// due to unprefixed relative imports being used, we whitelist the relative paths we use
// (?:$|\\/) matches end of string or /
[
'^(?:#\\/)?(?:lib|state|logger|platform|locale)(?:$|\\/)',
'^(?:#\\/)?view(?:$|\\/)',
'^(?:#\\/)?screens(?:$|\\/)',
'^(?:#\\/)?alf(?:$|\\/)',
'^(?:#\\/)?components(?:$|\\/)',
'^#\\/',
'^\\.',
],

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

2 participants