From e4dbc66dcc4f8a9c48e428e3de3b541dc1fecc5d Mon Sep 17 00:00:00 2001 From: Nicholas Shahan Date: Wed, 6 Jun 2018 12:06:46 -0700 Subject: [PATCH] Remove temp files and scratch space in favor of an Importer (#37) Add BuildImporter class to handle canonicalization and reading of imports. Add a check for `.sass` extension on inputId and use indented syntax. --- CHANGELOG.md | 7 ++ analysis_options.yaml | 1 + example/pubspec.yaml | 1 + lib/sass_builder.dart | 193 ++++-------------------------------- lib/src/build_importer.dart | 83 ++++++++++++++++ pubspec.yaml | 9 +- test/sass_builder_test.dart | 63 +++++------- 7 files changed, 141 insertions(+), 216 deletions(-) create mode 100644 lib/src/build_importer.dart diff --git a/CHANGELOG.md b/CHANGELOG.md index f1b70ab..111bf4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## 2.0.1 + +* Fix bug where `.sass` entrypoints were not being compiled with "indented" + syntax. +* Remove all temp file creation. Files are now imported using a custom + [AsyncImporter](https://github.com/sass/dart-sass/blob/0a9a503ae08b2e57b97d5e791024089986dd85c7/lib/src/importer/async.dart#L22). + ## 2.0.0 ### New Feature diff --git a/analysis_options.yaml b/analysis_options.yaml index 6834a4b..0a47e17 100644 --- a/analysis_options.yaml +++ b/analysis_options.yaml @@ -28,5 +28,6 @@ linter: - test_types_in_equals - throw_in_finally - type_init_formals + - unawaited_futures - unrelated_type_equality_checks - valid_regexps diff --git a/example/pubspec.yaml b/example/pubspec.yaml index 72d2beb..daa1465 100644 --- a/example/pubspec.yaml +++ b/example/pubspec.yaml @@ -6,5 +6,6 @@ dependencies: bootstrap_sass: 4.0.0-alpha.5+1 dev_dependencies: build_runner: ^0.8.8 + build_web_compilers: ^0.4.0 sass_builder: path: ../ diff --git a/lib/sass_builder.dart b/lib/sass_builder.dart index 261526b..9356fa4 100644 --- a/lib/sass_builder.dart +++ b/lib/sass_builder.dart @@ -1,23 +1,11 @@ import 'dart:async'; -import 'dart:collection'; import 'package:build/build.dart'; -import 'package:logging/logging.dart'; -import 'package:package_resolver/package_resolver.dart'; -import 'package:path/path.dart'; +import 'package:path/path.dart' as p; import 'package:sass/sass.dart' as sass; -import 'package:scratch_space/scratch_space.dart'; -final _packageNameRegExp = new RegExp(r'''package:([^\/]*)\/'''); -final _packagePathRegExp = new RegExp(r'''package:[^\/]*\/(.*)'''); -final _scssImportBlockRegExp = - new RegExp(r'''@import ([^;]*);''', multiLine: true); -final _sassImportBlockRegExp = - new RegExp(r'''@import (.*)$''', multiLine: true); -final _scssfileNameRegExp = new RegExp(r'''(?:\'|\")([^\'\"]*)(?:\'|\")'''); -final _sassfileNameRegExp = new RegExp(r'''['"]?([^ ,'"]+)['"]?'''); -final _scssCommentRegExp = - new RegExp(r'''//.*?\n|/\*.*?\*/''', multiLine: true); +import 'src/build_importer.dart'; + final outputStyleKey = 'outputStyle'; Builder sassBuilder(BuilderOptions options) => @@ -27,17 +15,10 @@ PostProcessBuilder sassSourceCleanup(BuilderOptions options) => new FileDeletingBuilder(['.scss', '.sass'], isEnabled: (options.config['enabled'] as bool) ?? false); -/// A `Builder` to compile .css files from .scss source using dart-sass. -/// -/// NOTE: Because Sass requires reading from the disk this `Builder` copies all -/// `Assets` to a temporary directory with a structure similar to that defined -/// in `.packages`. Sass will read from the temporary directory when compiling. +/// A `Builder` to compile `.css` files from `.scss` or `.sass` source using +/// the dart implementation of Sass. class SassBuilder implements Builder { - static final _scratchSpaceResource = new Resource( - () => new ScratchSpace(), - dispose: (temp) => temp.delete()); - - final _log = new Logger('sass_builder'); + static final _defaultOutputStyle = sass.OutputStyle.expanded; final String _outputExtension; final String _outputStyle; @@ -45,41 +26,35 @@ class SassBuilder implements Builder { : this._outputExtension = outputExtension, this._outputStyle = outputStyle ?? _defaultOutputStyle.toString(); - static final _defaultOutputStyle = sass.OutputStyle.expanded; - @override Future build(BuildStep buildStep) async { - var inputId = buildStep.inputId; + final inputId = buildStep.inputId; - if (basename(inputId.path).startsWith('_')) { + if (p.basename(inputId.path).startsWith('_')) { // Do not produce any output for .scss partials. - _log.fine('skipping partial file: ${inputId}'); + log.fine('skipping partial file: ${inputId}'); return; } - // Read and copy this asset and all imported assets to the temp directory. - _log.fine('processing file: ${inputId}'); - var tempDir = await buildStep.fetchResource(_scratchSpaceResource); - await _readAndCopyImports(inputId, buildStep, tempDir); - // Compile the css. - var tempAssetPath = tempDir.fileFor(inputId).path; - _log.fine('compiling file: ${tempAssetPath}'); - var cssOutput = sass.compile(tempAssetPath, - packageResolver: new SyncPackageResolver.root(tempDir.packagesDir.uri), + log.fine('compiling file: ${inputId.uri.toString()}'); + final cssOutput = await sass.compileStringAsync( + await buildStep.readAsString(inputId), + indented: inputId.extension == '.sass', + importers: [new BuildImporter(buildStep)], style: _getValidOutputStyle()); - // Write the builder output - var outputId = inputId.changeExtension(_outputExtension); - buildStep.writeAsString(outputId, '${cssOutput}\n'); - _log.fine('wrote css file: ${outputId.path}'); + // Write the builder output. + final outputId = inputId.changeExtension(_outputExtension); + await buildStep.writeAsString(outputId, '${cssOutput}\n'); + log.fine('wrote css file: ${outputId.path}'); } /// Returns a valid `OutputStyle` value to the `style` argument of - /// [sass.compile] during a [build]. + /// [sass.compileString] during a [build]. /// - /// * If [_outputStyle] is not `sass.OutputStyle.compressed` or - /// `sass.OutputStyle.expanded`, a warning will be logged informing the user + /// * If [_outputStyle] is not `OutputStyle.compressed` or + /// `OutputStyle.expanded`, a warning will be logged informing the user /// that the [_defaultOutputStyle] will be used. sass.OutputStyle _getValidOutputStyle() { if (_outputStyle == sass.OutputStyle.compressed.toString()) { @@ -87,7 +62,7 @@ class SassBuilder implements Builder { } else if (_outputStyle == sass.OutputStyle.expanded.toString()) { return sass.OutputStyle.expanded; } else { - _log.warning('Unknown outputStyle provided: "$_outputStyle". ' + log.warning('Unknown outputStyle provided: "$_outputStyle". ' 'Supported values are: "expanded" and "compressed". The default ' 'value of "${_defaultOutputStyle.toString()}" will be used.'); return _defaultOutputStyle; @@ -99,128 +74,4 @@ class SassBuilder implements Builder { '.scss': [_outputExtension], '.sass': [_outputExtension], }; - - // Reads `id` and all transitive imports while copying them to `tempDir`. - // - // Uses `buildStep to read the assets and the `ScratchSpace` API to write - // `tempDir`. - Future _readAndCopyImports( - AssetId id, BuildStep buildStep, ScratchSpace tempDir) async { - var copiedAssets = new Set(); - var assetsToCopy = new Queue(); - assetsToCopy.add(id); - - while (assetsToCopy.isNotEmpty) { - id = assetsToCopy.removeFirst(); - - if (!copiedAssets.contains(id)) { - var contents = await buildStep.readAsString(id); - _log.fine('read file: ${id}'); - await tempDir.ensureAssets([id], buildStep); - copiedAssets.add(id); - - var imports = await _importedAssets(id, contents, buildStep); - assetsToCopy.addAll(imports); - var importLog = imports.fold('', (acc, import) => '$acc\n ${import}'); - _log.fine('found imports:$importLog'); - } - } - } - - // Returns the `AssetId`s of all the scss imports in `contents`. - Future> _importedAssets( - AssetId id, String contents, BuildStep buildStep) async { - var importedAssets = new Set(); - - var importBlocks = id.extension == '.scss' - ? _scssImportBlockRegExp - .allMatches(contents.replaceAll(_scssCommentRegExp, '')) - : _sassImportBlockRegExp.allMatches(contents); - - for (var importBlock in importBlocks) { - var imports = id.extension == '.scss' - ? _scssfileNameRegExp.allMatches(importBlock.group(1)) - : _sassfileNameRegExp.allMatches(importBlock.group(1)); - for (var import in imports) { - var importId = await _findImport( - _importPackage(import.group(1), id.package), - _importPath(import.group(1), id.path), - buildStep); - if (importId == null) { - // Only copy imports that are found. If there is a problem with a - // missing file, let sass compilation fail and report it. - _log.warning('can not read file: ${import.group(1)}'); - continue; - } - - importedAssets.add(importId); - } - } - - return importedAssets; - } - - // Returns the package name parsed from the given `import` or defaults to - // `currentPackage`. - String _importPackage(String import, String currentPackage) => - import.startsWith('package:') - ? _packageNameRegExp.firstMatch(import).group(1) - : currentPackage; - - // Returns the path parsed from the given `import` or defaults to - // locating the file in the `currentPath`. - String _importPath(String import, String currentPath) => - import.startsWith('package:') - ? join('lib', _packagePathRegExp.firstMatch(import).group(1)) - : join(dirname(currentPath), import); - - // Locates the asset for `path` in `package` or returns null if unreadable. - // - // Probes for different versions of the path in case the file is a partial - // (leading underscore can be omitted) or if the extension is omitted per the - // SASS `@import` syntax. Tests for file readability via `buildStep`. - Future _findImport( - String package, String path, BuildStep buildStep) async { - var importId = new AssetId(package, path); - - if (await buildStep.canRead(importId)) { - // File was found as written in the import. - return importId; - } - - // Try as a partial. - var partialFile = new AssetId(package, _asPartial(path)); - if (await buildStep.canRead(partialFile)) { - return partialFile; - } - - // Try adding the .scss extension. - var scssPath = '$path.scss'; - var scssFile = new AssetId(package, scssPath); - if (await buildStep.canRead(scssFile)) { - return scssFile; - } - - partialFile = new AssetId(package, _asPartial(scssPath)); - if (await buildStep.canRead(partialFile)) { - return partialFile; - } - - // Try adding the .sass extension - var sassPath = '$path.sass'; - var sassFile = new AssetId(package, sassPath); - if (await buildStep.canRead(sassFile)) { - return sassFile; - } - - partialFile = new AssetId(package, _asPartial(sassPath)); - if (await buildStep.canRead(partialFile)) { - return partialFile; - } - - // No version of the filename was found. - return null; - } - - String _asPartial(String path) => join(dirname(path), '_${basename(path)}'); } diff --git a/lib/src/build_importer.dart b/lib/src/build_importer.dart new file mode 100644 index 0000000..5535ae0 --- /dev/null +++ b/lib/src/build_importer.dart @@ -0,0 +1,83 @@ +import 'dart:async'; + +import 'package:build/build.dart'; +import 'package:path/path.dart' as p; +import 'package:sass/sass.dart' as sass; + +/// A [sass.AsyncImporter] for use during a [BuildStep] that supports Dart +/// package imports of Sass files. +/// +/// All methods are heavily inspired by functions from for import priorities: +/// https://github.com/sass/dart-sass/blob/f8b2c9111c1d5a3c07c9c8c0828b92bd87c548c9/lib/src/importer/utils.dart +class BuildImporter extends sass.AsyncImporter { + final BuildStep _buildStep; + + BuildImporter(this._buildStep); + + @override + FutureOr canonicalize(Uri url) async => + (await _resolveImport(url.toString()))?.uri; + + @override + FutureOr load(Uri url) async { + final id = new AssetId.resolve(url.toString(), from: _buildStep.inputId); + final sourceMapId = id.addExtension('.map'); + return new sass.ImporterResult( + await _buildStep.readAsString(id), + sourceMapUrl: sourceMapId.uri, + indented: id.extension == '.sass', + ); + } + + /// Resolves [import] using the same logic as the filesystem importer. + /// + /// This tries to fill in extensions and partial prefixes and check if a + /// directory default. If no file can be found, it returns `null`. + Future _resolveImport(String import) async { + final extension = p.extension(import); + if (extension == '.sass' || extension == '.scss') { + return _exactlyOne(await _tryImport(import)); + } + + return _exactlyOne(await _tryImportWithExtensions(import)) ?? + _tryImportAsDirectory(import); + } + + /// Like [_tryImport], but checks both `.sass` and `.scss` extensions. + Future> _tryImportWithExtensions(String import) async => + await _tryImport(import + '.sass') + await _tryImport(import + '.scss'); + + /// Returns the [AssetId] for [import] and/or the partial with the same name, + /// if either or both exists. + /// + /// If neither exists, returns an empty list. + Future> _tryImport(String import) async { + final imports = []; + final partialId = new AssetId.resolve( + p.join(p.dirname(import), '_${p.basename(import)}'), + from: _buildStep.inputId); + if (await _buildStep.canRead(partialId)) imports.add(partialId); + final importId = new AssetId.resolve(import, from: _buildStep.inputId); + if (await _buildStep.canRead(importId)) imports.add(importId); + return imports; + } + + /// Returns the resolved index file for [import] if [import] is a directory + /// and the index file exists. + /// + /// Otherwise, returns `null`. + Future _tryImportAsDirectory(String import) async => + _exactlyOne(await _tryImportWithExtensions(p.join(import, 'index'))); + + /// If [imports] contains exactly one import [AssetId], returns that import. + /// + /// If it contains no assets, returns `null`. If it contains more than one, + /// throws an exception. + AssetId _exactlyOne(List imports) { + if (imports.isEmpty) return null; + if (imports.length == 1) return imports.first; + + throw new FormatException('It is not clear which file to import. Found:\n' + + imports.map((import) => ' ${p.prettyUri(import.uri)}').join('\n')); + } +} diff --git a/pubspec.yaml b/pubspec.yaml index 65d311d..50107a7 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass_builder -version: 2.0.0 +version: 2.0.1 authors: - Luis Vargas - Kevin Moore @@ -11,11 +11,8 @@ environment: dependencies: build: ^0.12.5 build_config: ^0.2.6 - logging: ^0.11.3 - package_resolver: ^1.0.2 path: ^1.4.1 - sass: ^1.0.0 - scratch_space: ^0.0.1 + sass: ^1.5.0 dev_dependencies: - build_test: '>=0.9.1 <0.11.0' + build_test: ^0.10.2 test: ^0.12.0 diff --git a/test/sass_builder_test.dart b/test/sass_builder_test.dart index 1a9525c..81aa876 100644 --- a/test/sass_builder_test.dart +++ b/test/sass_builder_test.dart @@ -4,13 +4,20 @@ import 'package:sass_builder/sass_builder.dart'; import 'package:test/test.dart'; void main() { - // These tests only verify which assets are read and written by the - // SassBuilder. This is because the inputs are being parsed to find what files - // they import. - // - // These tests do not verify any output as that is determined by the sass - // implementation. - group('file import parsing tests', () { + /// These tests only verify which assets are read and written by the + /// [SassBuilder]. This is to test the behaivor of the AsyncImporter used to + /// handle Dart package imports and perform all file IO through the + /// build_step. + /// + /// In some cases additional dependencies are created on files that do not + /// exists because Sass allows the user to ommit the file extensions `.scss` + /// or `.sass`, the partial prefix `_` or the file name altogether. These + /// tests avoid testing for those dependencies that are created when probing + /// for imports. + /// + /// These tests do not verify any output as that is determined by the Sass + /// implementation. + group('build IO tests', () { SassBuilder builder; InMemoryAssetWriter writer; InMemoryAssetReader reader; @@ -49,7 +56,7 @@ void main() { await runBuilder(builder, inputs.keys, reader, writer, null); expect(writer.assets.keys, equals([primary.changeExtension('.css')])); - expect(reader.assetsRead, unorderedEquals([primary, import])); + expect(reader.assetsRead, containsAll([primary, import])); }); test('one relative partial import simplified name', () async { @@ -66,9 +73,7 @@ void main() { await runBuilder(builder, inputs.keys, reader, writer, null); expect(writer.assets.keys, equals([primary.changeExtension('.css')])); - - expect(reader.assetsRead, contains(primary)); - expect(reader.assetsRead, contains(import)); + expect(reader.assetsRead, containsAll([primary, import])); }); test('one relative import', () async { @@ -90,8 +95,7 @@ void main() { primary.changeExtension('.css'), import.changeExtension('.css') ])); - expect(reader.assetsRead, contains(primary)); - expect(reader.assetsRead, contains(import)); + expect(reader.assetsRead, containsAll([primary, import])); }); test('one package import', () async { @@ -108,9 +112,7 @@ void main() { await runBuilder(builder, inputs.keys, reader, writer, null); expect(writer.assets.keys, equals([primary.changeExtension('.css')])); - - expect(reader.assetsRead, contains(primary)); - expect(reader.assetsRead, contains(import)); + expect(reader.assetsRead, containsAll([primary, import])); }); test('multiple imports in one block', () async { @@ -136,10 +138,7 @@ void main() { primary.changeExtension('.css'), import1.changeExtension('.css') ])); - - expect(reader.assetsRead, contains(primary)); - expect(reader.assetsRead, contains(import1)); - expect(reader.assetsRead, contains(import2)); + expect(reader.assetsRead, containsAll([primary, import1, import2])); }); test('multiple imports in multiple blocks', () async { @@ -165,10 +164,7 @@ void main() { primary.changeExtension('.css'), import1.changeExtension('.css') ])); - - expect(reader.assetsRead, contains(primary)); - expect(reader.assetsRead, contains(import1)); - expect(reader.assetsRead, contains(import2)); + expect(reader.assetsRead, containsAll([primary, import1, import2])); }); test('transitive imports', () async { @@ -194,9 +190,7 @@ void main() { import1.changeExtension('.css') ])); - expect(reader.assetsRead, contains(primary)); - expect(reader.assetsRead, contains(import1)); - expect(reader.assetsRead, contains(import2)); + expect(reader.assetsRead, containsAll([primary, import1, import2])); }); test('.sass file import parsing', () async { @@ -216,10 +210,7 @@ void main() { await runBuilder(builder, inputs.keys, reader, writer, null); expect(writer.assets.keys, equals([primary.changeExtension('.css')])); - - expect(reader.assetsRead, contains(primary)); - expect(reader.assetsRead, contains(import1)); - expect(reader.assetsRead, contains(import2)); + expect(reader.assetsRead, containsAll([primary, import1, import2])); }); test('.sass file imports in other packages', () async { @@ -240,10 +231,7 @@ void main() { await runBuilder(builder, inputs.keys, reader, writer, null); expect(writer.assets.keys, equals([primary.changeExtension('.css')])); - - expect(reader.assetsRead, contains(primary)); - expect(reader.assetsRead, contains(import1)); - expect(reader.assetsRead, contains(import2)); + expect(reader.assetsRead, containsAll([primary, import1, import2])); }); test('multiple .sass imports in multiple blocks', () async { @@ -265,10 +253,7 @@ void main() { expect(writer.assets.keys, unorderedEquals([primary.changeExtension('.css')])); - - expect(reader.assetsRead, contains(primary)); - expect(reader.assetsRead, contains(import1)); - expect(reader.assetsRead, contains(import2)); + expect(reader.assetsRead, containsAll([primary, import1, import2])); }); }); }