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

What to do about bootstrap 5.2 #268

Closed
gravitystorm opened this issue Jul 12, 2023 · 4 comments
Closed

What to do about bootstrap 5.2 #268

gravitystorm opened this issue Jul 12, 2023 · 4 comments

Comments

@gravitystorm
Copy link
Owner

See sass/sassc-rails#174 and in particular my comment at sass/sassc-rails#174 (comment)

So originally sass was a ruby project, then it became a C++ "libsass" which was available as sassc. Now the official implementation is in Dart.

This is a list of related ruby gems, there's a lot here:

  • sass - the og implementation of sass, in ruby
  • sassc - rubygem that uses libsass. Note that the repo is called sassc-ruby.
  • sass-rails - was originally based on sass, now just a wrapper for sassc-rails.
  • sassc-rails - what we use, depends on sassc and sprockets-rails
  • sass-embedded - a wrapper around the Dart version of sass. Note that the repo is sass-embedded-host-ruby
  • sassc-embedded - a shim to make sassc work with sass-embedded. Note that the repo is sassc-embedded-shim-ruby
  • dartsass-ruby - a wrapper around sass-embedded, but which implements sassc compatibility
  • dartsass-sprockets - a wrapper around dartsass-ruby, but which implements sassc-rails compatibility

The problem is that the bootstrap gem depends directly on sassc-rails, which makes it impossible to use dartsass-sprockets since they reuse the same namespace when both are loaded at the same time.

Option 1

Pin sassc gem to an old version, from before April 2019 when the error message was implemented in libsass.

  • This doesn't work because a different error is thrown.

Option 2

Use the sassc-embedded shim. This involves running a modified version of sassc that disables half the stuff, plus the sassc-embedded shim to make that stuff work via dart.

Option 3

Use dartsass-sprockets.

  • Is this possible somehow? I mean, we could try forking the bootstrap gem or something, but that sounds gnarly.

Option 4

Rename our css files to scss. This means that sprockets can do the stylesheets in one pass, which saves libsass from trying to read the minified version that is triggering this error.

Followups

  • Why does bootstrap-rubygem depend on sassc-rails at all? I would have thought it a resource for that gem to consume.
  • Is there any issues open in bootstrap itself for the maths deprecation warnings?
  • libsass doesn't support math functions, but dartsass will drop the / operator in 2.0. What is bootstrap itself going to do at that point?
@gravitystorm
Copy link
Owner Author

Final two points aren't relevant - bootstrap uses a custom division function that works with libsass, effectively "reverse polyfilling" support from the new dart version back to the old version.

The deprecation warnings come from our own code, which uses / in several places.

@gravitystorm
Copy link
Owner Author

I thought we'd solved this by switching around our includes, but it still triggers in precompilation. Latest review of options:

Option 2.1

  • Refactor our stylesheets to avoid using / (grep -rn '/[0-9]' app/assets/stylesheets/)
  • Then use the sassc-embedded shim

Option 3.1

  • Nurse https://github.com/twbs/bootstrap-rubygem/pull/256 across the line
  • Upgrade directly to whatever version that gets release in (or more likely, one or more forked versions with that patch applied, to enable incremental releases 5.2, 5.3 etc

Option 4.1

See if we can refactor further, so that all the stylesheets are handled in one pass, e.g. including leaflet and iD via sass @import instead of via sprockets stuff

Option 5

See if we can persuade sassc-rails to become a wrapper for dartsass-sprockets

Option 6

Figure out what alternatives there are for using bootstrap, e.g. not from a gem, to allow us to switch to dartsass-sprockets

Option 7

Figure out whether to drop sprockets and move to whatever CSS option is the default approach in rails.

@gravitystorm
Copy link
Owner Author

Option 3.1 has happened!

So now we need to do some forking of the gem, since I doubt it would be backported to existing releases. We'll need to start with a 5.1 fork that has the dependency removed, start using that with dartsass, and then do the same again for 5.2

@gravitystorm
Copy link
Owner Author

https://github.com/openstreetmap/openstreetmap-website/pull/4261

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

No branches or pull requests

1 participant