-
Notifications
You must be signed in to change notification settings - Fork 878
Modernize Bootstrap Sass: Migrate from @import to @use/@forward syntax #9123
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
base: master
Are you sure you want to change the base?
Conversation
|
@seuros thank your for spending time on the theme package, as this is quite a large blob of changes, I'm not sure yet when we can process these. Eventually it looks like we should deprecate the @import statements (https://sass-lang.com/blog/import-is-deprecated/) and changing the default bootstrap files is likely much easier than aiming for an upgrade anyway. |
|
Thank you for reviewing! I completely understand . I've actually been maintaining these changes locally for a while now, but having to reapply them after every OPNsense upgrade has become routine. So I figured it's time to contribute it upstream! The changes are entirely mechanical, no functional changes to the generated CSS, just modernizing the Sass syntax to use I'd be happy to also migrate all the plugin themes in the
If I migrate all the official themes, there's will be no extra work to be done, everything will just work and the deprecation warnings will be gone. I can submit a separate PR to the plugins repo once this is accepted. What i do is to use the migrator and check manually each page that get affected. I'm happy to:
No rush on my end. PS: the reason i put 1 PR/commit. is that in case there is some unseen regression, you just revert. But this is just CSS compilation, nothing is active here. |
|
no need to break it up, I just need some time to test and assess impact, I do compile these regularly, so I need to know for sure the output remains the same, that's all. The plugins are not maintained by the core team, so before spending time, I would advise to ask the maintainers when this is merged. Out of sheer curiosity, why would you like to compile our theme every release? |
|
Good question! I don't recompile every release - just major releases. This time it's for the newly released Viper. Over the years, I've built quite a few custom plugins on top of OPNsense (LoRa emergency messaging, 4G failover, custom UPS integration for older hardware that doesn't support NUT, etc.). Some of these actually conflict with OPNsense's enterprise offerings, and they're all highly optimized for my specific hardware setup. I also have a custom dashboard theme with full WebGL 3D network representation, heatmaps, etc. - but that breaks compatibility with non-Chrome browsers and mobile, so not very general-purpose. I'm not interested in open-sourcing most of these custom tools because:
I also make other customizations like removing legacy DHCP and aliasing everything to KEA. But the Bootstrap/Sass migration is different - it's not a custom feature, it's a general improvement that everyone will need eventually. The deprecation warnings affect all OPNsense installations, and Dart Sass 3.0 will break compilation entirely. That's why I'm contributing this specific fix upstream rather than keeping it in my private fork! |
…lor rendering. As mentioned in #9123, some statements will be deprecated. To lower the diff in future vresions, make sure we recompile the css files with the current compiler.
…es color rendering. As mentioned in #9123, some statements will be deprecated. To lower the diff in future vresions, make sure we recompile the css files with the current compiler.
This is my first attempt to give back to the community since I've been using OPNsense for nearly a decade. My router recently grew its own consciousness and told me to either contribute or get a license. So here is the Bootstrap upgrade I extracted from my port for the Greater Firewall of North Korea.
Technical Details
This upgrade removes the deprecated
@importstatements that were causing circular dependency nightmares and replaces them with the modern Sass module system (@useand@forward).Why this matters:
@importrule will be removed entirely in Dart Sass 3.0.0 (expected no sooner than October 2026). Without this migration, OPNsense themes will stop compiling when that version is released@importis officially deprecated and throws warnings@importsystem would eagerly load everything, causing variables to leak globally and creating a tangled web of dependencies@use, each file's variables and mixins are properly encapsulated, preventing naming collisions@use, unlike@importwhich would reload files multiple times!globalflags needed - everything stays properly scopedChanges Made
lighten()/darken()→color.adjust()red()/green()/blue()→color.change()math.percentage()/math.floor()@usedeclarations!optionalflags to@extendstatements to prevent compilation errorsTesting
Both themes compile successfully with only minor mixed-declarations warnings (future CSS cascade layer behavior, not critical):
P.S. Kidding about North Korea, it was Syria or Russia. Don't remember, I was offline firewalled behind OPNsense.