From c8505016214b28b9dbaffd5bec9d2ce3dc6ffaf7 Mon Sep 17 00:00:00 2001 From: Goodwine <2022649+Goodwine@users.noreply.github.com> Date: Thu, 18 Aug 2022 16:49:09 -0700 Subject: [PATCH] Allow a module loaded multiple times by the same configuration (#1739) * Allow a module loaded multiple times by the same configuration * use object references as opaque ID Fixes #1716 --- CHANGELOG.md | 3 ++ lib/src/configuration.dart | 54 +++++++++++++++++++++++++---- lib/src/visitor/async_evaluate.dart | 7 +++- lib/src/visitor/evaluate.dart | 9 +++-- 4 files changed, 63 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e9655233..0b4faa5f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ `random()` currently ignores units. A future version will no longer ignore units. +* Don't throw an error when the same module is `@forward`ed multiple times + through a configured module. + ## 1.54.4 * Improve error messages when passing incorrect units that are also diff --git a/lib/src/configuration.dart b/lib/src/configuration.dart index 04a3a8048..b8f10a5bb 100644 --- a/lib/src/configuration.dart +++ b/lib/src/configuration.dart @@ -3,7 +3,6 @@ // https://opensource.org/licenses/MIT. import 'dart:collection'; - import 'ast/node.dart'; import 'ast/sass.dart'; import 'configured_value.dart'; @@ -27,14 +26,39 @@ class Configuration { final Map _values; /// Creates an implicit configuration with the given [values]. - Configuration.implicit(this._values); + Configuration.implicit(this._values) : __originalConfiguration = null; + + /// The backing value for [_originalConfiguration]. + /// + /// This is null if [_originalConfiguration] refers to itself since `this` + /// can't be assigned to a final field. + final Configuration? __originalConfiguration; + + /// The configuration from which this was modified with `@forward ... with`. + /// + /// This reference serves as an opaque ID. + Configuration get _originalConfiguration => __originalConfiguration ?? this; + + /// Returns whether `this` and [that] [Configuration]s have the same + /// [_originalConfiguration]. + /// + /// An implicit configuration will always return `false` because it was not + /// created through another configuration. + /// + /// [ExplicitConfiguration]s will and configurations created [throughForward] + /// will be considered to have the same original config if they were created + /// as a copy from the same base configuration. + bool sameOriginal(Configuration that) => + _originalConfiguration == that._originalConfiguration; /// The empty configuration, which indicates that the module has not been /// configured. /// /// Empty configurations are always considered implicit, since they are /// ignored if the module has already been loaded. - const Configuration.empty() : _values = const {}; + const Configuration.empty() + : _values = const {}, + __originalConfiguration = null; bool get isEmpty => values.isEmpty; @@ -65,9 +89,15 @@ class Configuration { return _withValues(newValues); } - /// Returns a copy of [this] with the given [values] map. + /// Returns a copy of `this` [Configuration] with the given [values] map. + /// + /// The copy will have the same [_originalConfiguration] as `this` config. Configuration _withValues(Map values) => - Configuration.implicit(values); + Configuration._(values, _originalConfiguration); + + /// Creates a [Configuration] with the given [_values] map and an + /// [_originalConfiguration] reference. + Configuration._(this._values, this.__originalConfiguration); String toString() => "(" + @@ -88,10 +118,20 @@ class ExplicitConfiguration extends Configuration { /// The node whose span indicates where the configuration was declared. final AstNode nodeWithSpan; + /// Creates a base [ExplicitConfiguration] with a [values] map and a + /// [nodeWithSpan]. ExplicitConfiguration(Map values, this.nodeWithSpan) : super.implicit(values); - /// Returns a copy of [this] with the given [values] map. + /// Creates an [ExplicitConfiguration] with a [values] map, a [nodeWithSpan] + /// and if this is a copy a reference to the [_originalConfiguration]. + ExplicitConfiguration._(Map values, + this.nodeWithSpan, Configuration? originalConfiguration) + : super._(values, originalConfiguration); + + /// Returns a copy of `this` with the given [values] map. + /// + /// The copy will have the same [_originalConfiguration] as `this` config. Configuration _withValues(Map values) => - ExplicitConfiguration(values, nodeWithSpan); + ExplicitConfiguration._(values, nodeWithSpan, _originalConfiguration); } diff --git a/lib/src/visitor/async_evaluate.dart b/lib/src/visitor/async_evaluate.dart index f6d0c9553..50e62b723 100644 --- a/lib/src/visitor/async_evaluate.dart +++ b/lib/src/visitor/async_evaluate.dart @@ -141,6 +141,9 @@ class _EvaluateVisitor /// All modules that have been loaded and evaluated so far. final _modules = {}; + /// The first [Configuration] used to load a module [Uri]. + final _moduleConfigurations = {}; + /// A map from canonical module URLs to the nodes whose spans indicate where /// those modules were originally loaded. /// @@ -670,7 +673,8 @@ class _EvaluateVisitor var alreadyLoaded = _modules[url]; if (alreadyLoaded != null) { var currentConfiguration = configuration ?? _configuration; - if (currentConfiguration is ExplicitConfiguration) { + if (!_moduleConfigurations[url]!.sameOriginal(currentConfiguration) && + currentConfiguration is ExplicitConfiguration) { var message = namesInErrors ? "${p.prettyUri(url)} was already loaded, so it can't be " "configured using \"with\"." @@ -751,6 +755,7 @@ class _EvaluateVisitor var module = environment.toModule(css, extensionStore); if (url != null) { _modules[url] = module; + _moduleConfigurations[url] = _configuration; if (nodeWithSpan != null) _moduleNodes[url] = nodeWithSpan; } diff --git a/lib/src/visitor/evaluate.dart b/lib/src/visitor/evaluate.dart index b02630d28..1f74f7c5a 100644 --- a/lib/src/visitor/evaluate.dart +++ b/lib/src/visitor/evaluate.dart @@ -5,7 +5,7 @@ // DO NOT EDIT. This file was generated from async_evaluate.dart. // See tool/grind/synchronize.dart for details. // -// Checksum: c3bc020b07dc2376f57ef8e9990ff6b97cde3417 +// Checksum: ad5691d0c80b924308f6ee03863fa8c7f5e25ca0 // // ignore_for_file: unused_import @@ -149,6 +149,9 @@ class _EvaluateVisitor /// All modules that have been loaded and evaluated so far. final _modules = >{}; + /// The first [Configuration] used to load a module [Uri]. + final _moduleConfigurations = {}; + /// A map from canonical module URLs to the nodes whose spans indicate where /// those modules were originally loaded. /// @@ -675,7 +678,8 @@ class _EvaluateVisitor var alreadyLoaded = _modules[url]; if (alreadyLoaded != null) { var currentConfiguration = configuration ?? _configuration; - if (currentConfiguration is ExplicitConfiguration) { + if (!_moduleConfigurations[url]!.sameOriginal(currentConfiguration) && + currentConfiguration is ExplicitConfiguration) { var message = namesInErrors ? "${p.prettyUri(url)} was already loaded, so it can't be " "configured using \"with\"." @@ -756,6 +760,7 @@ class _EvaluateVisitor var module = environment.toModule(css, extensionStore); if (url != null) { _modules[url] = module; + _moduleConfigurations[url] = _configuration; if (nodeWithSpan != null) _moduleNodes[url] = nodeWithSpan; }