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

[RFC] Implement support for built-in Sass modules #421

Open
stof opened this issue May 28, 2021 · 5 comments
Open

[RFC] Implement support for built-in Sass modules #421

stof opened this issue May 28, 2021 · 5 comments

Comments

@stof
Copy link
Member

stof commented May 28, 2021

Implemented full support for modules (tracked in #55) will be a huge work. This RFC suggests an incremental process where we first support only built-in modules (the sass:* modules defined by the compiler) without supporting userland modules.

Rationale

New Sass functions are only exposed as part of modules, without a global alias. That's totally understandable as any global function risks conflicts with future CSS functions. This means that if we don't ship built-in modules, we cannot ship these new functions.
With dart-sass shipping the first part of the migration towards / being a separator (see https://sass-lang.com/documentation/breaking-changes/slash-div), not being able to ship math.div() means it is not possible to write code compatible with scssphp without getting deprecation warnings in dart-sass (see #419).

I expect it to be much simpler to support built-in modules than to have full support for modules:

  • built-in modules cannot be configured
  • built-in modules don't output any CSS, and so are not impacted by the evaluate-once semantic of modules which changes how CSS is emitted (we could even consider that built-in modules are not evaluated at all as they are part of the compiler)
  • built-in modules don't need to implement a separate scope to evaluate them (as they are not actually evaluated)

This would still force to write your own code with @import to be compatible (but that's not deprecated in dart-sass), until full support lands.

Things to implement

  • parser support for namespaces in variable access and function/mixin calls
  • parser support for @use, but triggering errors for any non-builtin modules
  • evaluation support for module functions for functions we already have
  • implementation of functions existing only in modules (sass:math has a lot of them)
  • update of our sass-spec runner so that it does not automatically skip tests using @use. Instead, it should only do it when the @use is for a userland module (and for @forward). Any unsupported test using a builtin module can still be skipped by the exclude list of course.

@forward won't be implemented in this step, as I don't see any reason to forward one of the built-in module. It will be implemented only as part of the full implementation.

@robocoder
Copy link
Member

LGTM.

Would you maintain a feature branch until the first built-in module/function can be parsed and evaluated, or would you release things (eg parser changes) as they're implemented?

Is there a prioritized list of built-in modules/functions to be implemented? Or should it be driven by issues and PRs?

@stof
Copy link
Member Author

stof commented May 28, 2021

Is there a prioritized list of built-in modules/functions to be implemented? Or should it be driven by issues and PRs?

Well, I would probably implement it first for functions that we already have through their global alias (because we already have the implementation). Then, I haven't prioritized the list of new functions yet, but math.div() is probably very high in the list (due to the / migration). Then, I would probably work module by module (there are only 7 built-in modules).
What I'm not sure yet is whether I treat #266 as a prerequisite.

Would you maintain a feature branch until the first built-in module/function can be parsed and evaluated, or would you release things (eg parser changes) as they're implemented?

I already have a local branch where I started rewriting the Parser from scratch as a port of the dart-sass one (I started that when figuring out that fixing the parsing of interpolation in our existing parser was a nightmare). In that (partial) rewrite, I already handle the parsing of namespaces when producing the AST, except that I trigger an error instead of returning the namespaced AST. But that work is big (and I figured out I need to change the parsing of selectors first if I want to split the work, as selectors must be parsed after interpolation)
I haven't investigated yet how hard it would be to introduce the parsing of namespaces in our existing parser, to decide whether this RFC can be implemented before I finish my parser rewrite (as I just started the thinking on that RFC). But if it can be brought in the current parser, I would probably follow the same approach: implement the full parsing of namespaces triggering an error at the end of that parsing in case of namespaces, then implementing support for @use but triggering an error with all modules being not found, then implementing the evaluation support for namespaces, then actually bringing the implementation of built-in modules. Each step could then be merged (and potentially released) without waiting for the full work. I'd rather avoid a long-running feature branch if I can.

@matteo-greco
Copy link

Hello everyone. Do you have any news regarding possibly supporting Sass modules? I'm working on a project that makes massive use of them and I'm pondering our what our next move should be.

@stof
Copy link
Member Author

stof commented Jan 4, 2022

My focus right now (on the part of my free time that I dedicate to this project) is on version 2.0 using the new architecture based on the dart-sass one. The new parser is done, but not the new evaluator.
I gave up on adding support for parsing namespaces in the 1.x branch for now, because the Parser code is a huge mess. So there is no progress on supporting Sass modules yet.

@Kidokyfe

This comment was marked as off-topic.

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

4 participants