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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 2 commits into from Oct 22, 2019
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
01e03f1
Reset the forwarded config to empty for @use and meta.load-module()
nex3 Oct 18, 2019
6e30f78
Merge branch 'master' of github.com:sass/dart-sass into config-bug
nex3 Oct 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -3,6 +3,9 @@
* Fix a bug preventing built-in modules from being loaded within a configured
module.

* Fix a bug preventing an unconfigured module from being loaded from within two
different configured modules.

* Fix a bug when `meta.load-css()` was used to load some files that included
media queries.

Expand Down
20 changes: 10 additions & 10 deletions lib/src/visitor/async_evaluate.dart
View file
Open in desktop
Expand Up @@ -413,9 +413,9 @@ class _EvaluateVisitor
var url = Uri.parse(arguments[0].assertString("module").text);
var withMap = arguments[1].realNull?.assertMap("with")?.contents;

Map<String, _ConfiguredValue> configuration;
var configuration = const <String, _ConfiguredValue>{};
if (withMap != null) {
configuration = <String, _ConfiguredValue>{};
configuration = {};
var span = _callableNode.span;
withMap.forEach((variable, value) {
var name =
Expand Down Expand Up @@ -525,11 +525,9 @@ class _EvaluateVisitor
{Uri baseUrl,
Map<String, _ConfiguredValue> configuration,
bool namesInErrors = false}) async {
configuration ??= const {};

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.

throw _exception(
namesInErrors
? "Built-in module $url can't be configured."
Expand Down Expand Up @@ -589,12 +587,11 @@ class _EvaluateVisitor
Future<Module> _execute(AsyncImporter importer, Stylesheet stylesheet,
{Map<String, _ConfiguredValue> configuration,
bool namesInErrors = false}) async {
configuration ??= const {};
var url = stylesheet.span.sourceUrl;

var alreadyLoaded = _modules[url];
if (alreadyLoaded != null) {
if (configuration.isNotEmpty || _configuration.isNotEmpty) {
if ((configuration ?? _configuration).isNotEmpty) {
throw _exception(namesInErrors
? "${p.prettyUri(url)} was already loaded, so it can't be "
"configured using \"with\"."
Expand Down Expand Up @@ -637,7 +634,10 @@ class _EvaluateVisitor
_atRootExcludingStyleRule = false;
_inKeyframes = false;

if (configuration.isNotEmpty) _configuration = Map.of(configuration);
if (configuration != null) {
_configuration =
configuration.isEmpty ? const {} : Map.of(configuration);
}

await visitStylesheet(stylesheet);
css = _outOfOrderImports == null
Expand All @@ -658,7 +658,7 @@ class _EvaluateVisitor
_atRootExcludingStyleRule = oldAtRootExcludingStyleRule;
_inKeyframes = oldInKeyframes;

if (configuration.isNotEmpty && _configuration.isNotEmpty) {
if (configuration != null && _configuration.isNotEmpty) {
throw _exception(
namesInErrors
? "\$${_configuration.keys.first} was not declared with "
Expand Down Expand Up @@ -1780,7 +1780,7 @@ class _EvaluateVisitor
_environment.addModule(module, namespace: node.namespace);
},
configuration: node.configuration.isEmpty
? null
? const {}
: {
for (var entry in node.configuration.entries)
entry.key: _ConfiguredValue(
Expand Down
22 changes: 11 additions & 11 deletions lib/src/visitor/evaluate.dart
View file
Open in desktop
Expand Up @@ -5,7 +5,7 @@
// DO NOT EDIT. This file was generated from async_evaluate.dart.
// See tool/grind/synchronize.dart for details.
//
// Checksum: 9083313a65eb24875e7292b859202fd574daf46b
// Checksum: 8abca72a6d671844aa9eb075aef56f58fd71ddf7
//
// ignore_for_file: unused_import

Expand Down Expand Up @@ -419,9 +419,9 @@ class _EvaluateVisitor
var url = Uri.parse(arguments[0].assertString("module").text);
var withMap = arguments[1].realNull?.assertMap("with")?.contents;

Map<String, _ConfiguredValue> configuration;
var configuration = const <String, _ConfiguredValue>{};
if (withMap != null) {
configuration = <String, _ConfiguredValue>{};
configuration = {};
var span = _callableNode.span;
withMap.forEach((variable, value) {
var name =
Expand Down Expand Up @@ -531,11 +531,9 @@ class _EvaluateVisitor
{Uri baseUrl,
Map<String, _ConfiguredValue> configuration,
bool namesInErrors = false}) {
configuration ??= const {};

var builtInModule = _builtInModules[url];
if (builtInModule != null) {
if (configuration.isNotEmpty) {
if (configuration != null && configuration.isNotEmpty) {
throw _exception(
namesInErrors
? "Built-in module $url can't be configured."
Expand Down Expand Up @@ -595,12 +593,11 @@ class _EvaluateVisitor
Module<Callable> _execute(Importer importer, Stylesheet stylesheet,
{Map<String, _ConfiguredValue> configuration,
bool namesInErrors = false}) {
configuration ??= const {};
var url = stylesheet.span.sourceUrl;

var alreadyLoaded = _modules[url];
if (alreadyLoaded != null) {
if (configuration.isNotEmpty || _configuration.isNotEmpty) {
if ((configuration ?? _configuration).isNotEmpty) {
throw _exception(namesInErrors
? "${p.prettyUri(url)} was already loaded, so it can't be "
"configured using \"with\"."
Expand Down Expand Up @@ -643,7 +640,10 @@ class _EvaluateVisitor
_atRootExcludingStyleRule = false;
_inKeyframes = false;

if (configuration.isNotEmpty) _configuration = Map.of(configuration);
if (configuration != null) {
_configuration =
configuration.isEmpty ? const {} : Map.of(configuration);
}

visitStylesheet(stylesheet);
css = _outOfOrderImports == null
Expand All @@ -664,7 +664,7 @@ class _EvaluateVisitor
_atRootExcludingStyleRule = oldAtRootExcludingStyleRule;
_inKeyframes = oldInKeyframes;

if (configuration.isNotEmpty && _configuration.isNotEmpty) {
if (configuration != null && _configuration.isNotEmpty) {
throw _exception(
namesInErrors
? "\$${_configuration.keys.first} was not declared with "
Expand Down Expand Up @@ -1774,7 +1774,7 @@ class _EvaluateVisitor
_environment.addModule(module, namespace: node.namespace);
},
configuration: node.configuration.isEmpty
? null
? const {}
: {
for (var entry in node.configuration.entries)
entry.key: _ConfiguredValue(
Expand Down