diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b2ccc65f..eaf6dcc39 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## 1.90.0 + +* Allow a `@forward`ed module to be loaded with a configuration when that module + has already been loaded with a different configuration *and* the module + doesn't define any variables that would have been configured anyway. + ## 1.89.2 ### Embedded Host diff --git a/lib/src/async_environment.dart b/lib/src/async_environment.dart index b2b9f6521..fa8458ab4 100644 --- a/lib/src/async_environment.dart +++ b/lib/src/async_environment.dart @@ -121,6 +121,10 @@ final class AsyncEnvironment { UserDefinedCallable? get content => _content; UserDefinedCallable? _content; + /// The set of variable names that could be configured when loading the + /// module. + final Set _configurableVariables; + /// Whether the environment is lexically at the root of the document. bool get atRoot => _variables.length == 1; @@ -160,27 +164,28 @@ final class AsyncEnvironment { _functions = [{}], _functionIndices = {}, _mixins = [{}], - _mixinIndices = {}; + _mixinIndices = {}, + _configurableVariables = {}; AsyncEnvironment._( - this._modules, - this._namespaceNodes, - this._globalModules, - this._importedModules, - this._forwardedModules, - this._nestedForwardedModules, - this._allModules, - this._variables, - this._variableNodes, - this._functions, - this._mixins, - this._content, - ) - // Lazily fill in the indices rather than eagerly copying them from the - // existing environment in closure() because the copying took a lot of - // time and was rarely helpful. This saves a bunch of time on Susy's - // tests. - : _variableIndices = {}, + this._modules, + this._namespaceNodes, + this._globalModules, + this._importedModules, + this._forwardedModules, + this._nestedForwardedModules, + this._allModules, + this._variables, + this._variableNodes, + this._functions, + this._mixins, + this._content, + this._configurableVariables) + // Lazily fill in the indices rather than eagerly copying them from the + // existing environment in closure() because the copying took a lot of + // time and was rarely helpful. This saves a bunch of time on Susy's + // tests. + : _variableIndices = {}, _functionIndices = {}, _mixinIndices = {}; @@ -202,6 +207,9 @@ final class AsyncEnvironment { _functions.toList(), _mixins.toList(), _content, + // Closures are always in nested contexts where configurable variables + // are never added. + const {}, ); /// Returns a new environment to use for an imported file. @@ -222,6 +230,7 @@ final class AsyncEnvironment { _functions.toList(), _mixins.toList(), _content, + _configurableVariables, ); /// Adds [module] to the set of modules visible in this environment. @@ -640,6 +649,15 @@ final class AsyncEnvironment { _variableNodes[index][name] = nodeWithSpan; } + /// Records that [name] is a variable that could have been configured for this + /// module, whether or not it actually was. + /// + /// This is used to determine whether to throw an error when passing a new + /// configuration through `@forward` to an already-loaded module. + void markVariableConfigurable(String name) { + _configurableVariables.add(name); + } + /// Returns the value of the function named [name], optionally with the given /// [namespace], or `null` if no such variable is declared. /// @@ -1070,6 +1088,26 @@ final class _EnvironmentModule implements Module { return module == null ? this : module.variableIdentity(name); } + bool couldHaveBeenConfigured(Set variables) => + // Check if this module defines a configurable variable with any of the + // given names. + (variables.length < _environment._configurableVariables.length + ? variables.any(_environment._configurableVariables.contains) + : _environment._configurableVariables.any(variables.contains)) || + // Find forwarded modules whose variables overlap with [variables] and + // check if they define configurable variables with any of the given + // names. + (variables.length < _modulesByVariable.length + ? { + for (var variable in variables) + if (_modulesByVariable[variable] case var module?) module + } + : { + for (var (variable, module) in _modulesByVariable.pairs) + if (variables.contains(variable)) module + }) + .any((module) => module.couldHaveBeenConfigured(variables)); + Module cloneCss() { if (!transitivelyContainsCss) return this; diff --git a/lib/src/configuration.dart b/lib/src/configuration.dart index fb4138481..3cc5b05fd 100644 --- a/lib/src/configuration.dart +++ b/lib/src/configuration.dart @@ -50,7 +50,7 @@ final class Configuration { /// 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; + identical(_originalConfiguration, that._originalConfiguration); /// The empty configuration, which indicates that the module has not been /// configured. @@ -70,7 +70,7 @@ final class Configuration { /// Creates a new configuration from this one based on a `@forward` rule. Configuration throughForward(ForwardRule forward) { - if (isEmpty) return const Configuration.empty(); + if (isEmpty) return this; var newValues = _values; // Only allow variables that are visible through the `@forward` to be diff --git a/lib/src/environment.dart b/lib/src/environment.dart index 5312ad71f..63c82ae53 100644 --- a/lib/src/environment.dart +++ b/lib/src/environment.dart @@ -5,7 +5,7 @@ // DO NOT EDIT. This file was generated from async_environment.dart. // See tool/grind/synchronize.dart for details. // -// Checksum: add8a3972aec53ef29de3639af2bc84edcbde9e7 +// Checksum: 72b802e4004aae8a84f7aa78ec728861339b846b // // ignore_for_file: unused_import @@ -128,6 +128,10 @@ final class Environment { UserDefinedCallable? get content => _content; UserDefinedCallable? _content; + /// The set of variable names that could be configured when loading the + /// module. + final Set _configurableVariables; + /// Whether the environment is lexically at the root of the document. bool get atRoot => _variables.length == 1; @@ -167,27 +171,28 @@ final class Environment { _functions = [{}], _functionIndices = {}, _mixins = [{}], - _mixinIndices = {}; + _mixinIndices = {}, + _configurableVariables = {}; Environment._( - this._modules, - this._namespaceNodes, - this._globalModules, - this._importedModules, - this._forwardedModules, - this._nestedForwardedModules, - this._allModules, - this._variables, - this._variableNodes, - this._functions, - this._mixins, - this._content, - ) - // Lazily fill in the indices rather than eagerly copying them from the - // existing environment in closure() because the copying took a lot of - // time and was rarely helpful. This saves a bunch of time on Susy's - // tests. - : _variableIndices = {}, + this._modules, + this._namespaceNodes, + this._globalModules, + this._importedModules, + this._forwardedModules, + this._nestedForwardedModules, + this._allModules, + this._variables, + this._variableNodes, + this._functions, + this._mixins, + this._content, + this._configurableVariables) + // Lazily fill in the indices rather than eagerly copying them from the + // existing environment in closure() because the copying took a lot of + // time and was rarely helpful. This saves a bunch of time on Susy's + // tests. + : _variableIndices = {}, _functionIndices = {}, _mixinIndices = {}; @@ -209,6 +214,9 @@ final class Environment { _functions.toList(), _mixins.toList(), _content, + // Closures are always in nested contexts where configurable variables + // are never added. + const {}, ); /// Returns a new environment to use for an imported file. @@ -229,6 +237,7 @@ final class Environment { _functions.toList(), _mixins.toList(), _content, + _configurableVariables, ); /// Adds [module] to the set of modules visible in this environment. @@ -648,6 +657,15 @@ final class Environment { _variableNodes[index][name] = nodeWithSpan; } + /// Records that [name] is a variable that could have been configured for this + /// module, whether or not it actually was. + /// + /// This is used to determine whether to throw an error when passing a new + /// configuration through `@forward` to an already-loaded module. + void markVariableConfigurable(String name) { + _configurableVariables.add(name); + } + /// Returns the value of the function named [name], optionally with the given /// [namespace], or `null` if no such variable is declared. /// @@ -1080,6 +1098,26 @@ final class _EnvironmentModule implements Module { return module == null ? this : module.variableIdentity(name); } + bool couldHaveBeenConfigured(Set variables) => + // Check if this module defines a configurable variable with any of the + // given names. + (variables.length < _environment._configurableVariables.length + ? variables.any(_environment._configurableVariables.contains) + : _environment._configurableVariables.any(variables.contains)) || + // Find forwarded modules whose variables overlap with [variables] and + // check if they define configurable variables with any of the given + // names. + (variables.length < _modulesByVariable.length + ? { + for (var variable in variables) + if (_modulesByVariable[variable] case var module?) module + } + : { + for (var (variable, module) in _modulesByVariable.pairs) + if (variables.contains(variable)) module + }) + .any((module) => module.couldHaveBeenConfigured(variables)); + Module cloneCss() { if (!transitivelyContainsCss) return this; diff --git a/lib/src/module.dart b/lib/src/module.dart index b545b2751..5a8dd9a55 100644 --- a/lib/src/module.dart +++ b/lib/src/module.dart @@ -30,8 +30,7 @@ abstract interface class Module { /// [AstNode.span] if the span isn't required, since some nodes need to do /// real work to manufacture a source span. /// - /// Implementations must ensure that this has the same keys as [variables] if - /// it's not `null`. + /// Implementations must ensure that this has the same keys as [variables]. Map get variableNodes; /// The module's functions. @@ -81,6 +80,10 @@ abstract interface class Module { /// question, as defined by the Sass spec. Object variableIdentity(String name); + /// Whether this module exposes any variables from among [variables] that + /// could have been configured when the module was loaded. + bool couldHaveBeenConfigured(Set variables); + /// Creates a copy of this module with new [css] and [extender]. Module cloneCss(); } diff --git a/lib/src/module/built_in.dart b/lib/src/module/built_in.dart index 991bfcfba..39995db10 100644 --- a/lib/src/module/built_in.dart +++ b/lib/src/module/built_in.dart @@ -62,5 +62,7 @@ final class BuiltInModule implements Module { return this; } + bool couldHaveBeenConfigured(Set _) => false; + Module cloneCss() => this; } diff --git a/lib/src/module/forwarded_view.dart b/lib/src/module/forwarded_view.dart index 2875490f3..445016e1b 100644 --- a/lib/src/module/forwarded_view.dart +++ b/lib/src/module/forwarded_view.dart @@ -140,6 +140,31 @@ class ForwardedModuleView implements Module { return _inner.variableIdentity(name); } + bool couldHaveBeenConfigured(Set variables) { + assert(_rule.shownVariables == null || _rule.hiddenVariables == null); + if (_rule.prefix == null && + _rule.shownVariables == null && + (_rule.hiddenVariables?.isEmpty ?? true)) { + return _inner.couldHaveBeenConfigured(variables); + } + + if (_rule.prefix case var prefix?) { + variables = { + for (var name in variables) + if (name.startsWith(prefix)) name.substring(prefix.length) + }; + } + + if (_rule.shownVariables case var safelist?) { + return _inner.couldHaveBeenConfigured(variables.intersection(safelist)); + } else if (_rule.hiddenVariables case var blocklist? + when blocklist.isNotEmpty) { + return _inner.couldHaveBeenConfigured(variables.difference(blocklist)); + } else { + return _inner.couldHaveBeenConfigured(variables); + } + } + bool operator ==(Object other) => other is ForwardedModuleView && _inner == other._inner && diff --git a/lib/src/module/shadowed_view.dart b/lib/src/module/shadowed_view.dart index 32144b402..cf84455a0 100644 --- a/lib/src/module/shadowed_view.dart +++ b/lib/src/module/shadowed_view.dart @@ -107,6 +107,14 @@ final class ShadowedModuleView implements Module { return _inner.variableIdentity(name); } + bool couldHaveBeenConfigured(Set variables) => + this.variables == _inner.variables + ? _inner.couldHaveBeenConfigured(variables) + : _inner.couldHaveBeenConfigured({ + for (var name in this.variables.keys) + if (variables.contains(name)) name + }); + bool operator ==(Object other) => other is ShadowedModuleView && _inner == other._inner && diff --git a/lib/src/visitor/async_evaluate.dart b/lib/src/visitor/async_evaluate.dart index c1db0e72e..8e31e7280 100644 --- a/lib/src/visitor/async_evaluate.dart +++ b/lib/src/visitor/async_evaluate.dart @@ -854,10 +854,18 @@ final class _EvaluateVisitor }) async { var url = stylesheet.span.sourceUrl; + var currentConfiguration = configuration ?? _configuration; if (_modules[url] case var alreadyLoaded?) { - var currentConfiguration = configuration ?? _configuration; if (!_moduleConfigurations[url]!.sameOriginal(currentConfiguration) && - currentConfiguration is ExplicitConfiguration) { + currentConfiguration is ExplicitConfiguration && + // Don't throw an error if the module being loaded doesn't expose any + // configurable variables that could have been affected by the + // configuration in the first place. If the configuration defines a + // value that's not in the module, it'll still throw an error, but + // this avoids throwing confusing errors for `@forward`ed modules + // without configuration. + alreadyLoaded.couldHaveBeenConfigured( + MapKeySet(currentConfiguration.values))) { var message = namesInErrors ? "${p.prettyUri(url)} was already loaded, so it can't be " "configured using \"with\"." @@ -946,7 +954,7 @@ final class _EvaluateVisitor ); if (url != null) { _modules[url] = module; - _moduleConfigurations[url] = _configuration; + _moduleConfigurations[url] = currentConfiguration; if (nodeWithSpan != null) _moduleNodes[url] = nodeWithSpan; } @@ -2599,6 +2607,7 @@ final class _EvaluateVisitor Future visitVariableDeclaration(VariableDeclaration node) async { if (node.isGuarded) { if (node.namespace == null && _environment.atRoot) { + _environment.markVariableConfigurable(node.name); if (_configuration.remove(node.name) case var override? when override.value != sassNull) { _addExceptionSpan(node, () { diff --git a/lib/src/visitor/evaluate.dart b/lib/src/visitor/evaluate.dart index 5727e56aa..11420063c 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: a3068d04660dd2bed34b884aa6e1a21d423dc4e5 +// Checksum: 70d926fd13a7d35cd805bf7053f073cc123b260f // // ignore_for_file: unused_import @@ -862,10 +862,18 @@ final class _EvaluateVisitor }) { var url = stylesheet.span.sourceUrl; + var currentConfiguration = configuration ?? _configuration; if (_modules[url] case var alreadyLoaded?) { - var currentConfiguration = configuration ?? _configuration; if (!_moduleConfigurations[url]!.sameOriginal(currentConfiguration) && - currentConfiguration is ExplicitConfiguration) { + currentConfiguration is ExplicitConfiguration && + // Don't throw an error if the module being loaded doesn't expose any + // configurable variables that could have been affected by the + // configuration in the first place. If the configuration defines a + // value that's not in the module, it'll still throw an error, but + // this avoids throwing confusing errors for `@forward`ed modules + // without configuration. + alreadyLoaded.couldHaveBeenConfigured( + MapKeySet(currentConfiguration.values))) { var message = namesInErrors ? "${p.prettyUri(url)} was already loaded, so it can't be " "configured using \"with\"." @@ -954,7 +962,7 @@ final class _EvaluateVisitor ); if (url != null) { _modules[url] = module; - _moduleConfigurations[url] = _configuration; + _moduleConfigurations[url] = currentConfiguration; if (nodeWithSpan != null) _moduleNodes[url] = nodeWithSpan; } @@ -2604,6 +2612,7 @@ final class _EvaluateVisitor Value? visitVariableDeclaration(VariableDeclaration node) { if (node.isGuarded) { if (node.namespace == null && _environment.atRoot) { + _environment.markVariableConfigurable(node.name); if (_configuration.remove(node.name) case var override? when override.value != sassNull) { _addExceptionSpan(node, () { diff --git a/pkg/sass-parser/CHANGELOG.md b/pkg/sass-parser/CHANGELOG.md index ef9e27284..1c1862ee3 100644 --- a/pkg/sass-parser/CHANGELOG.md +++ b/pkg/sass-parser/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.4.25 + +* No user-visible changes. + ## 0.4.24 * No user-visible changes. diff --git a/pkg/sass-parser/package.json b/pkg/sass-parser/package.json index f3d816655..128c4b1e2 100644 --- a/pkg/sass-parser/package.json +++ b/pkg/sass-parser/package.json @@ -1,6 +1,6 @@ { "name": "sass-parser", - "version": "0.4.24", + "version": "0.4.25", "description": "A PostCSS-compatible wrapper of the official Sass parser", "repository": "sass/sass", "author": "Google Inc.", diff --git a/pkg/sass_api/CHANGELOG.md b/pkg/sass_api/CHANGELOG.md index e034f36f9..aed8ba3e8 100644 --- a/pkg/sass_api/CHANGELOG.md +++ b/pkg/sass_api/CHANGELOG.md @@ -1,3 +1,7 @@ +## 15.8.0 + +* No user-visible changes. + ## 15.7.1 * No user-visible changes. diff --git a/pkg/sass_api/pubspec.yaml b/pkg/sass_api/pubspec.yaml index a0ad29aca..729f75849 100644 --- a/pkg/sass_api/pubspec.yaml +++ b/pkg/sass_api/pubspec.yaml @@ -2,7 +2,7 @@ name: sass_api # Note: Every time we add a new Sass AST node, we need to bump the *major* # version because it's a breaking change for anyone who's implementing the # visitor interface(s). -version: 15.7.1 +version: 15.8.0 description: Additional APIs for Dart Sass. homepage: https://github.com/sass/dart-sass @@ -10,7 +10,7 @@ environment: sdk: ">=3.6.0 <4.0.0" dependencies: - sass: 1.89.2 + sass: 1.90.0 dev_dependencies: dartdoc: ^8.0.14 diff --git a/pubspec.yaml b/pubspec.yaml index 039be9860..41e3a07bc 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass -version: 1.89.2 +version: 1.90.0 description: A Sass implementation in Dart. homepage: https://github.com/sass/dart-sass