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

implement first class mixins #2073

Merged
merged 36 commits into from
Oct 5, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
ef23769
initial impl of first class mixins
connorskees Aug 25, 2023
0b9f3a8
add mixin type to js api
connorskees Aug 26, 2023
63c1f5e
add first class mixins to embedded protocol
connorskees Aug 26, 2023
f9badfa
support AsyncBuiltInCallable in accepts-content
connorskees Aug 26, 2023
010ad35
generate evaluate file
connorskees Aug 26, 2023
b908e27
bump ci
connorskees Aug 29, 2023
7d6757c
merge main
connorskees Sep 1, 2023
f715c22
revert embedded protocol changes
connorskees Sep 2, 2023
c0eced3
Style nits
nex3 Sep 6, 2023
91ac279
some pr review
connorskees Sep 7, 2023
194e171
Revert "revert embedded protocol changes"
connorskees Sep 7, 2023
154e12e
remove redundant content cast
connorskees Sep 8, 2023
ca57cb0
correctly pass content through meta.apply
connorskees Sep 8, 2023
4093914
correctly pass through acceptsContent value
connorskees Sep 8, 2023
eb33c56
pr review
connorskees Sep 14, 2023
eeb14ee
refactor embedded registry
connorskees Sep 14, 2023
1cfd14b
merge main
connorskees Sep 14, 2023
5e1774c
merge main
connorskees Sep 16, 2023
faba24a
await _applyMixin
connorskees Sep 19, 2023
52a1e6f
Merge branch 'main' of https://github.com/sass/dart-sass into feat/fi…
connorskees Sep 20, 2023
ff3d80e
Merge branch 'main' of https://github.com/sass/dart-sass into feat/fi…
connorskees Sep 21, 2023
89ef731
add doc comment to _applyMixin
connorskees Sep 21, 2023
93b812a
run synchronize
connorskees Sep 22, 2023
6e85d50
bump ci
connorskees Sep 25, 2023
2738c00
merge main
connorskees Sep 26, 2023
80fd8bb
Update pubspec and changelog
nex3 Sep 28, 2023
9485b4f
merge main
connorskees Sep 29, 2023
3144e02
nits
connorskees Sep 30, 2023
f1c1d5a
lazily evaluate file spans
connorskees Oct 3, 2023
895b2ca
Merge branch 'main' of https://github.com/sass/dart-sass into feat/fi…
connorskees Oct 3, 2023
96bbca2
bump ci
connorskees Oct 3, 2023
679e1bd
bump ci
connorskees Oct 4, 2023
65f2d2d
Merge branch 'main' of https://github.com/sass/dart-sass into feat/fi…
connorskees Oct 4, 2023
75ccdfb
take Object self in JS constructor
connorskees Oct 4, 2023
6277d5d
bump ci
connorskees Oct 4, 2023
f1b4ff3
add SassMixin to ESM export
connorskees Oct 5, 2023
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
9 changes: 6 additions & 3 deletions lib/src/callable/built_in.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ final class BuiltInCallable implements Callable, AsyncBuiltInCallable {
/// The overloads declared for this callable.
final List<(ArgumentDeclaration, Callback)> _overloads;

bool hasContent = false;
connorskees marked this conversation as resolved.
Show resolved Hide resolved

/// Creates a function with a single [arguments] declaration and a single
/// [callback].
///
Expand Down Expand Up @@ -48,18 +50,19 @@ final class BuiltInCallable implements Callable, AsyncBuiltInCallable {
/// defined.
BuiltInCallable.mixin(
String name, String arguments, void callback(List<Value> arguments),
{Object? url})
{Object? url, bool hasContent = false})
: this.parsed(name,
ArgumentDeclaration.parse('@mixin $name($arguments) {', url: url),
(arguments) {
callback(arguments);
return sassNull;
});
}, hasContent: hasContent);

/// Creates a callable with a single [arguments] declaration and a single
/// [callback].
BuiltInCallable.parsed(this.name, ArgumentDeclaration arguments,
Value callback(List<Value> arguments))
Value callback(List<Value> arguments),
{this.hasContent = false})
: _overloads = [(arguments, callback)];

/// Creates a function with multiple implementations.
Expand Down
4 changes: 3 additions & 1 deletion lib/src/embedded/dispatcher.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import 'host_callable.dart';
import 'importer/file.dart';
import 'importer/host.dart';
import 'logger.dart';
import 'mixin_registry.dart';
import 'util/proto_extensions.dart';
import 'utils.dart';

Expand Down Expand Up @@ -126,6 +127,7 @@ final class Dispatcher {
Future<OutboundMessage_CompileResponse> _compile(
InboundMessage_CompileRequest request) async {
var functions = FunctionRegistry();
var mixins = MixinRegistry();

var style = request.style == OutputStyle.COMPRESSED
? sass.OutputStyle.compressed
Expand All @@ -139,7 +141,7 @@ final class Dispatcher {
(throw mandatoryError("Importer.importer")));

var globalFunctions = request.globalFunctions
.map((signature) => hostCallable(this, functions, signature));
.map((signature) => hostCallable(this, functions, mixins, signature));

late sass.CompileResult result;
switch (request.whichInput()) {
Expand Down
8 changes: 5 additions & 3 deletions lib/src/embedded/host_callable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
// ignore: deprecated_member_use
import 'dart:cli';

import 'package:sass/src/embedded/mixin_registry.dart';

import '../callable.dart';
import '../exception.dart';
import 'dispatcher.dart';
Expand All @@ -21,12 +23,12 @@ import 'utils.dart';
/// the name defined in the [signature].
///
/// Throws a [SassException] if [signature] is invalid.
Callable hostCallable(
Dispatcher dispatcher, FunctionRegistry functions, String signature,
Callable hostCallable(Dispatcher dispatcher, FunctionRegistry functions,
MixinRegistry mixins, String signature,
{int? id}) {
late Callable callable;
callable = Callable.fromSignature(signature, (arguments) {
var protofier = Protofier(dispatcher, functions);
var protofier = Protofier(dispatcher, functions, mixins);
var request = OutboundMessage_FunctionCallRequest()
..arguments.addAll(
[for (var argument in arguments) protofier.protofy(argument)]);
Expand Down
33 changes: 33 additions & 0 deletions lib/src/embedded/mixin_registry.dart
connorskees marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Copyright 2019 Google Inc. Use of this source code is governed by an
// MIT-style license that can be found in the LICENSE file or at
// https://opensource.org/licenses/MIT.

import '../value/mixin.dart';
import 'embedded_sass.pb.dart';

/// A registry of [SassMixin]s indexed by ID so that the host can invoke
/// them.
final class MixinRegistry {
/// First-class mixins that have been sent to the host.
///
/// The mixins are located at indexes in the list matching their IDs.
final _mixinsById = <SassMixin>[];

/// A reverse map from mixins to their indexes in [_mixinsById].
final _idsByMixin = <SassMixin, int>{};

/// Converts [mixin] to a protocol buffer to send to the host.
Value_CompilerMixin protofy(SassMixin mixin) {
var id = _idsByMixin.putIfAbsent(mixin, () {
_mixinsById.add(mixin);
return _mixinsById.length - 1;
});

return Value_CompilerMixin()..id = id;
}

/// Returns the compiler-side mixin associated with [id].
///
/// If no such mixin exists, returns `null`.
SassMixin? operator [](int id) => _mixinsById[id];
}
19 changes: 17 additions & 2 deletions lib/src/embedded/protofier.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import 'dispatcher.dart';
import 'embedded_sass.pb.dart' as proto;
import 'embedded_sass.pb.dart' hide Value, ListSeparator, CalculationOperator;
import 'function_registry.dart';
import 'mixin_registry.dart';
import 'host_callable.dart';
import 'utils.dart';

Expand All @@ -23,6 +24,9 @@ final class Protofier {
/// The IDs of first-class functions.
final FunctionRegistry _functions;

/// The IDs of first-class mixins.
final MixinRegistry _mixins;

/// Any argument lists transitively contained in [value].
///
/// The IDs of the [Value_ArgumentList] protobufs are always one greater than
Expand All @@ -35,7 +39,10 @@ final class Protofier {
///
/// The [functions] tracks the IDs of first-class functions so that the host
/// can pass them back to the compiler.
Protofier(this._dispatcher, this._functions);
///
/// Similarly, the [mixins] tracks the IDs of first-class mixins so that the
/// host can pass them back to the compiler.
Protofier(this._dispatcher, this._functions, this._mixins);

/// Converts [value] to its protocol buffer representation.
proto.Value protofy(Value value) {
Expand Down Expand Up @@ -85,6 +92,8 @@ final class Protofier {
result.calculation = _protofyCalculation(value);
case SassFunction():
result.compilerFunction = _functions.protofy(value);
case SassMixin():
result.compilerMixin = _mixins.protofy(value);
case sassTrue:
result.singleton = SingletonValue.TRUE;
case sassFalse:
Expand Down Expand Up @@ -240,9 +249,15 @@ final class Protofier {

case Value_Value.hostFunction:
return SassFunction(hostCallable(
_dispatcher, _functions, value.hostFunction.signature,
_dispatcher, _functions, _mixins, value.hostFunction.signature,
id: value.hostFunction.id));

case Value_Value.compilerMixin:
var id = value.compilerMixin.id;
if (_mixins[id] case var mixin?) return mixin;
throw paramsError(
"CompilerMixin.id $id doesn't match any known mixins");

case Value_Value.calculation:
return _deprotofyCalculation(value.calculation);

Expand Down
13 changes: 13 additions & 0 deletions lib/src/functions/meta.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'dart:collection';

import 'package:collection/collection.dart';

import '../ast/sass/statement/mixin_rule.dart';
import '../callable.dart';
import '../util/map.dart';
import '../value.dart';
Expand Down Expand Up @@ -45,6 +46,7 @@ final global = UnmodifiableListView([
sassNull => "null",
SassNumber() => "number",
SassFunction() => "function",
SassMixin() => "mixin",
SassCalculation() => "calculation",
SassString() => "string",
_ => throw "[BUG] Unknown value type ${arguments[0]}"
Expand Down Expand Up @@ -77,6 +79,17 @@ final local = UnmodifiableListView([
? argument
: SassString(argument.toString(), quotes: false)),
ListSeparator.comma);
}),
_function("accepts-content", r"$mixin", (arguments) {
var mixin = arguments[0].assertMixin("mixin");
bool acceptsContent = switch (mixin.callable) {
BuiltInCallable(hasContent: var hasContent) => hasContent,
UserDefinedCallable(declaration: MixinRule(hasContent: var hasContent)) =>
hasContent,
AsyncCallable() =>
throw UnsupportedError("Unknown callable type $mixin."),
connorskees marked this conversation as resolved.
Show resolved Hide resolved
};
return SassBoolean(acceptsContent);
})
]);

Expand Down
1 change: 1 addition & 0 deletions lib/src/js.dart
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ void main() {
exports.CalculationInterpolation = calculationInterpolationClass;
exports.SassColor = colorClass;
exports.SassFunction = functionClass;
exports.SassMixin = mixinClass;
exports.SassList = listClass;
exports.SassMap = mapClass;
exports.SassNumber = numberClass;
Expand Down
1 change: 1 addition & 0 deletions lib/src/js/exports.dart
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class Exports {
external set SassBoolean(JSClass function);
external set SassColor(JSClass function);
external set SassFunction(JSClass function);
external set SassMixin(JSClass mixin);
external set SassList(JSClass function);
external set SassMap(JSClass function);
external set SassNumber(JSClass function);
Expand Down
2 changes: 2 additions & 0 deletions lib/src/js/value.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export 'value/color.dart';
export 'value/function.dart';
export 'value/list.dart';
export 'value/map.dart';
export 'value/mixin.dart';
export 'value/number.dart';
export 'value/string.dart';

Expand Down Expand Up @@ -42,6 +43,7 @@ final JSClass valueClass = () {
'assertColor': (Value self, [String? name]) => self.assertColor(name),
'assertFunction': (Value self, [String? name]) => self.assertFunction(name),
'assertMap': (Value self, [String? name]) => self.assertMap(name),
'assertMixin': (Value self, [String? name]) => self.assertMixin(name),
'assertNumber': (Value self, [String? name]) => self.assertNumber(name),
'assertString': (Value self, [String? name]) => self.assertString(name),
'tryMap': (Value self) => self.tryMap(),
Expand Down
22 changes: 22 additions & 0 deletions lib/src/js/value/mixin.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright 2021 Google Inc. Use of this source code is governed by an
// MIT-style license that can be found in the LICENSE file or at
// https://opensource.org/licenses/MIT.

import 'package:node_interop/js.dart';

import '../../callable.dart';
import '../../value.dart';
import '../reflection.dart';
import '../utils.dart';

/// The JavaScript `SassMixin` class.
final JSClass mixinClass = () {
var jsClass = createJSClass('sass.SassMixin', () {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if it is related to the browser failure, but I think the argument is missing Object self here when looking at all other values.

jsThrow(JsError(
'It is not possible to construct a SassMixin through the JavaScript API'));
});

getJSClass(SassMixin(Callable('f', '', (_) => sassNull)))
.injectSuperclass(jsClass);
Comment on lines +19 to +20
Copy link
Contributor

@ntkme ntkme Oct 4, 2023

Choose a reason for hiding this comment

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

Also this does not make much sense. If I understand it correctly the reason we have this for Sass Function type is to allow the "JS" type to inherit the prototype of dart-converted-JS type? If so here the argument passed into SassMixin (the dart type) should be some argument necessary to construct an actual Mixin object.

@nex3 I don't fully understand how the JS-interop works so please correct me if I'm wrong.

Copy link
Contributor

Choose a reason for hiding this comment

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

Never mind. Looks like Mixin just takes AsyncCallable so this is probably fine.

return jsClass;
}();
9 changes: 9 additions & 0 deletions lib/src/value.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import 'value/color.dart';
import 'value/function.dart';
import 'value/list.dart';
import 'value/map.dart';
import 'value/mixin.dart';
import 'value/number.dart';
import 'value/string.dart';
import 'visitor/interface/value.dart';
Expand All @@ -27,6 +28,7 @@ export 'value/color.dart';
export 'value/function.dart';
export 'value/list.dart';
export 'value/map.dart';
export 'value/mixin.dart';
export 'value/null.dart';
export 'value/number.dart' hide conversionFactor;
export 'value/string.dart';
Expand Down Expand Up @@ -177,6 +179,13 @@ abstract class Value {
SassFunction assertFunction([String? name]) =>
throw SassScriptException("$this is not a function reference.", name);

/// Throws a [SassScriptException] if [this] isn't a mixin reference.
///
/// If this came from a mixin argument, [name] is the argument name
connorskees marked this conversation as resolved.
Show resolved Hide resolved
/// (without the `$`). It's used for error reporting.
SassMixin assertMixin([String? name]) =>
throw SassScriptException("$this is not a mixin reference.", name);

/// Throws a [SassScriptException] if [this] isn't a map.
///
/// If this came from a function argument, [name] is the argument name
Expand Down
38 changes: 38 additions & 0 deletions lib/src/value/mixin.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright 2016 Google Inc. Use of this source code is governed by an
// MIT-style license that can be found in the LICENSE file or at
// https://opensource.org/licenses/MIT.

import 'package:meta/meta.dart';

import '../callable.dart';
import '../visitor/interface/value.dart';
import '../value.dart';

/// A SassScript mixin reference.
///
/// A mixin reference captures a mixin from the local environment so that
/// it may be passed between modules.
///
/// {@category Value}
@sealed
connorskees marked this conversation as resolved.
Show resolved Hide resolved
class SassMixin extends Value {
/// The callable that this mixin invokes.
///
/// Note that this is typed as an [AsyncCallable] so that it will work with
/// both synchronous and asynchronous evaluate visitors, but in practice the
/// synchronous evaluate visitor will crash if this isn't a [Callable].
final AsyncCallable callable;
connorskees marked this conversation as resolved.
Show resolved Hide resolved

SassMixin(this.callable);

/// @nodoc
@internal
T accept<T>(ValueVisitor<T> visitor) => visitor.visitMixin(this);

SassMixin assertMixin([String? name]) => this;

bool operator ==(Object other) =>
other is SassMixin && callable == other.callable;

int get hashCode => callable.hashCode;
}