Error checking of `@use`/`@forward` ... with... is overly strict

[arikorn]

Sass does not allow a previously-loaded module to be loaded later using a with clause. I.e. the following is illegal:

// bad.scss

@use "module1";
// the following is not allowed:
@use 'module1' with ( $white: #bbb);

However, when numerous interdependent modules are loaded from a single @use call, such as when loading the popular framework, CoreUI, the Sass error-checking algorithm can lead to spurious errors because even modules that are not affected by the overriding variables are flagged as if they were called with a with clause. See Issue #2598 for several simplified examples.
Just to illustrate the magnitude of the issue, one might load CoreUI, for example, with a few variables overridden:

//Top-level
@use '@coreui/coreui/scss/coreui' with ( $white: #bbb);

This loads the following file (shown in part):

// @coreui/coreui/scss/coreui.scss
@forward "variables";
@forward "variables-dark";
@forward "functions";
@forward "mixins";
....  // loads around 50 files, some of which load even more modules

In this example, only the file '_variables.scss' will have a variable overridden by the top-level call. The remaining files are completely unaffected and do not need to be treated as if they are being loaded with a with clause. Currently, however, Error: This module was already loaded, so it can't be configured using "with". errors can be generated despite the fact the the with was a no-op for those files.

A much better approach would be to only flag files that actually have variables overridden. I will submit a PR shortly, which demonstrates this idea.

Note: As a side-effect this also improves error-reporting when the cause of the error is due to having specified a non-existent variable in the with clause. See examples in Issue #2598

[nex3]

There are two major issues here. First, I'm not convinced how desirable it actually would be to change this behavior. Right now, the way Sass behaves follows straightforward rules:

1. You may not pass configuration to an already-loaded module.
2. @forward passes the configuration from the current module into the forwarded module.

I understand that this can impose some annoying ordering restrictions on libraries, but the benefit of having clearly-explainable behavior is a substantial counterweight to that concern. In addition, there are fairly straightforward workarounds, such as reordering the loads so that each module is @forwarded (and thus properly configured) before it's @used, or using a more robust configuration API as the documentation recommends for complex use-cases.

The second issue is that I don't think your proposed fix actually accomplishes what it would need to even if a change here were desirable. To "only flag files that actually have variables overridden" fundamentally doesn't work as the logic here, because it's important that the following is an error:

@use "other" as _;
@use "other" with ($var: value);

The second @use absolutely should raise an error, because the user is expecting to receive a configured module, but the first @use didn't override any variables. (Under your current PR, this actually throws a bogus A much better approach would be to only flag files that actually have variables overridden. error instead.) The only principled way to address this would be to determine whether a module is configurable with the current configuration even if it's already been loaded, and as I mentioned in #2598 (comment) tracking the set of configurable variables after a module was loaded would likely be more expensive than it would be worth.

[arikorn]

Thank you, Natalie (@nex3), for sharing your expertise and understanding of Sass (and I hope you had a great 4th of July!). Your scss examples, especially, have been really excellent in helping me understand how the code works and what needs to be done. And yes, I now see why it's necessary to determine whether a module is configur_able_ with the current configuration even if it's already been loaded. I believe I've fixed it on my side and will update the PR shortly.

So we likely have a "way" to implement this; the remaining question is if there's a "will" to implement the idea described in this issue! Here's my thinking:

We both agreed in #2598 that the error message that I reported is confusing.

I'm sure we both agree that introducing breaking changes should not be done lightly (i.e. PR #2600).

I'm certain we both agree that improving modularity is a good thing (after all that's why @import is deprecated. In fact, the consequence of "modernizing" the @import rules in Bitfocus Companion's scss, is what led to my reporting issue #2598)

This issue and PR #2602 improve the modularity of Sass by

1. Ensuring that no module is affected by variables that are not part of the module.
2. Making scss/sass files less sensitive to the order of loading (as would be expected in a modular system)

Neither of these changes seem confusing, rather, they make Sass more robust and -- at least to a developer like me -- less confusing! (TBH, even with your explanation in #2598, I couldn't explain to someone else why an error was generated...and I have a PhD and many years of programming experience! This issue and PR simply makes it not an error, as I would have expected based on my understanding of modularity.)

As a side-effect, this idea and PR #2602 , may reduce the "breakage" consequences of PR #2600, or maybe even make it unnecessary.

One last thing, on a practical note from my experience using Sass with CoreUI: CoreUI will transpile and work perfectly on its own. As things stand, and even more so with what you said PR #2600 would do, the errors only pop-up when a developer tries to override variables defined in CoreUI -- something that is a fundamental feature! With this issue/PR, the problem goes away (and error messages correctly report incorrectly-included variables). This is particularly helpful since developers have somewhat limited recourse to fixing issues in external modules. As mentioned in issue #2598, my example there is taken directly from CoreUI, which claims on their webpage to be used by millions of developers.

[arikorn]

Note: This conversation continues in PR #2602