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

Fix performance regression from selector spans #1916

Merged
merged 2 commits into from
Mar 14, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
6 changes: 5 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,11 @@ jobs:
dart_channel: [stable]
node_version: [18]
include:
# Temporarily adding back Node 12 here until we actually drop support
# in the package.json.
- os: ubuntu-latest
dart_channel: stable
node_version: 12
# Include LTS versions on Ubuntu
- os: ubuntu-latest
dart_channel: stable
Expand All @@ -150,7 +155,6 @@ jobs:
- os: ubuntu-latest
dart_channel: dev
node_version: 18

steps:
- uses: actions/checkout@v3
- uses: dart-lang/setup-dart@v1
Expand Down
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
## 1.59.3

* Fix a performance regression introduced in 1.59.0.

* The NPM release of 1.59.0 dropped support for Node 12 without actually
indicating so in its pubspec. This release temporarily adds back support so
that the latest Sass version that declares it supports Node 12 actually does
so. However, Node 12 is now end-of-life, so we will drop support for it
properly in an upcoming release.

## 1.59.2

* No user-visible changes.
Expand Down
6 changes: 5 additions & 1 deletion lib/src/parse/parser.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import '../exception.dart';
import '../interpolation_map.dart';
import '../logger.dart';
import '../util/character.dart';
import '../util/lazy_file_span.dart';
import '../utils.dart';

/// The abstract base class for all parsers.
Expand Down Expand Up @@ -675,7 +676,10 @@ class Parser {
@protected
FileSpan spanFrom(LineScannerState state) {
var span = scanner.spanFrom(state);
return _interpolationMap?.mapSpan(span) ?? span;
if (_interpolationMap != null) {
return LazyFileSpan(() => _interpolationMap!.mapSpan(span));
}
return span;
}

/// Prints a warning to standard error, associated with [span].
Expand Down
58 changes: 58 additions & 0 deletions lib/src/util/lazy_file_span.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// Copyright 2023 Google LLC. 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:source_span/source_span.dart';

/// A wrapper for [FileSpan] that allows an expensive creation process to be
/// deferred until the span is actually needed.
class LazyFileSpan implements FileSpan {
/// The function that creates the underlying span.
final FileSpan Function() _builder;

/// The underlying span this wraps, which is created the first time this
/// getter is referenced.
FileSpan get span => _span ??= _builder();
FileSpan? _span;

/// Creates a new [LazyFileSpan] that defers calling [builder] until the
/// underlying span is needed.
LazyFileSpan(FileSpan Function() builder) : _builder = builder;

@override
int compareTo(SourceSpan other) => span.compareTo(other);

@override
String get context => span.context;

@override
FileLocation get end => span.end;

@override
FileSpan expand(FileSpan other) => span.expand(other);

@override
SourceFile get file => span.file;

@override
String highlight({color}) => span.highlight(color: color);

@override
int get length => span.length;

@override
String message(String message, {color}) =>
span.message(message, color: color);

@override
Uri? get sourceUrl => span.sourceUrl;

@override
FileLocation get start => span.start;

@override
String get text => span.text;

@override
SourceSpan union(SourceSpan other) => span.union(other);
}
4 changes: 4 additions & 0 deletions pkg/sass_api/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 6.0.3

* No user-visible changes.

## 6.0.2

* No user-visible changes.
Expand Down
4 changes: 2 additions & 2 deletions pkg/sass_api/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ 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: 6.0.2
version: 6.0.3
description: Additional APIs for Dart Sass.
homepage: https://github.com/sass/dart-sass

environment:
sdk: ">=2.17.0 <3.0.0"

dependencies:
sass: 1.59.2
sass: 1.59.3

dev_dependencies:
dartdoc: ^5.0.0
Expand Down
2 changes: 1 addition & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ dependencies:
dev_dependencies:
analyzer: ^4.7.0
archive: ^3.1.2
cli_pkg: ^2.1.4
cli_pkg: ^2.4.2
crypto: ^3.0.0
dart_style: ^2.0.0
dartdoc: ^6.0.0
Expand Down