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

Decouple from sassc #25

Closed
brodock opened this issue Oct 19, 2022 · 5 comments
Closed

Decouple from sassc #25

brodock opened this issue Oct 19, 2022 · 5 comments

Comments

@brodock
Copy link

brodock commented Oct 19, 2022

It would be nice to import code from sassc so we don't have to use a git branch everywhere.

I understand this was made likely to make easier to migrate without breaking everyone's dependency, but we could cut a major release that does not require to add:

gem 'sassc', github: 'sass/sassc-ruby', ref: 'refs/pull/233/head'

This can make it easier for gems to target this new approach.

@brodock
Copy link
Author

brodock commented Oct 21, 2022

@ntkme if you agree I would love to help and perhaps provide a MR with the proposal above. Ideally this would be done and released as part of sass/sassc-ruby but until we have them take action this is the second best option, please consider it.

@johnnyshields
Copy link

@brodock I'm working on it now. Going to release as a new gem.

@johnnyshields
Copy link

johnnyshields commented Oct 21, 2022

@brodock raised PR to SassC Ruby actually: sass/sassc-ruby#240. All tests passing, still double-checking.

@ntkme
Copy link
Member

ntkme commented Oct 24, 2022

The main problem is that many apps have sassc as dependency and require 'sassc' in dependency code that they don't have any control over. While it is possible to also have lib/sassc.rb in this project, the $LOAD_PATH order is not guaranteed, which is why this currently relies on a fork of sassc so that the shim code can be loaded whenever original sassc is required.

With that said, the only way to further improve current user experience is what @johnnyshields is attempting to do, which is to make the shim the actual upstream implementation. However, I don't really think upstream would support it.

Even if the upstream is willing to take this as sassc 3.x, there are quite a few issues. Dart Sass made the decision to revamp the importer behavior and which is very different from sassc. In Dart Sass, an @import or @use directive will strictly map to only one file, whereas in sassc, it used to be possible use glob like @import 'modules/*' to import multiple files. In order to support this, sassc-embedded uses a custom importer that generates an intermediate stylesheet to import all the files resolved from a glob pattern. This is not only discouraged as it defeats the purpose of the new module system (@use), and will be broken once Sass drops support of @import in the future. There are a few more things made more strict in Dart Sass that sassc-embedded has they loosen to be compatible with sassc, for example, to allow importing files without extension name, which can be a security issue depends on system configuration. Other than importers, there's the issue with custom functions in sassc that functions are registered as globals rather than locals, leading to thread safety problems like rails/sprockets#759, and eventually affect performance even after the fix. Not to mention no way to access keyword arguments and other new features not being supported (not possible without breaking existing code).

My take is that sassc-embedded is nothing but a stopgap solution to buy some time and let users in the ruby ecosystem migrate their Sass/Scss code to the latest Sass implementation with minimum effort.

The shim is not meant to be the long term solution, as the sass-embedded gem with the new Ruby API closely mirroring the new JS API from upstream is the future.

@ntkme ntkme closed this as not planned Won't fix, can't repro, duplicate, stale Oct 24, 2022
@ntkme
Copy link
Member

ntkme commented Oct 24, 2022

TL;DR: If you care about the experience, please directly integrate with sass-embedded or ask the dependency you use to do so. So far I have had success with jekyll-sass-converter and tilt now that they natively support sass-embedded without shim. I would encourage the community to contribute such integration in other projects.

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

3 participants