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

Reset the forwarded config to empty for @use and meta.load-module() #855

Merged
merged 2 commits into from Oct 22, 2019

Conversation

nex3
Copy link
Contributor

@nex3 nex3 commented Oct 18, 2019

In #827 I made configuration never be null, but this turns out to be
overkill: we still need a way for _loadModule() and _execute() to
distinguish between "use the existing configuration" (for @forward)
and "use no configuration" (for an unconfigured @use or
meta.load-module()). We now use null as a sentinel value there, while
still ensuring that _configuration is non-nullable.

Closes #854
See sass/sass-spec#1486

In #827 I made configuration never be null, but this turns out to be
overkill: we still need a way for _loadModule() and _execute() to
distinguish between "use the existing configuration" (for @forward)
and "use no configuration" (for an unconfigured @use or
meta.load-module()). We now use null as a sentinel value there, while
still ensuring that _configuration is non-nullable.

Closes #854
@nex3 nex3 requested a review from jathak October 18, 2019 22:34
var builtInModule = _builtInModules[url];
if (builtInModule != null) {
if (configuration.isNotEmpty) {
if (configuration != null && configuration.isNotEmpty) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would the better style here and in line 661 be configuration?.isNotEmpty ?? false?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered that, but I think the null logic is easier to follow here than trying to follow a bunch of ?s. It's close, though.

@nex3 nex3 merged commit 1b17ab7 into master Oct 22, 2019
@nex3 nex3 deleted the config-bug branch October 22, 2019 00:35
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

Successfully merging this pull request may close these issues.

Reusing a module with @use and arguments
2 participants