From cc431f38cd22231870d941cc141f1fe42e955bd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Wed, 20 Sep 2023 17:07:23 -0700 Subject: [PATCH] Breaking change on importer function signature --- lib/sass/canonicalize_context.rb | 20 ++ lib/sass/embedded.rb | 1 + lib/sass/embedded/host/importer_registry.rb | 20 +- lib/sass/embedded/protofier.rb | 7 + spec/sass_importer_spec.rb | 354 ++++++++++++++++++-- 5 files changed, 376 insertions(+), 26 deletions(-) create mode 100644 lib/sass/canonicalize_context.rb diff --git a/lib/sass/canonicalize_context.rb b/lib/sass/canonicalize_context.rb new file mode 100644 index 00000000..42ecd4ca --- /dev/null +++ b/lib/sass/canonicalize_context.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module Sass + # Contextual information passed to `canonicalize` and `find_file_url`. + # Not all importers will need this information to resolve loads, but some may find it useful. + # + # @see https://sass-lang.com/documentation/js-api/interfaces/canonicalizecontext/ + class CanonicalizeContext + # @return [String, nil] + attr_reader :containing_url + + # @return [Boolean] + attr_reader :from_import + + def initialize(containing_url, from_import) + @containing_url = containing_url + @from_import = from_import + end + end +end diff --git a/lib/sass/embedded.rb b/lib/sass/embedded.rb index 672c550e..d78043c1 100644 --- a/lib/sass/embedded.rb +++ b/lib/sass/embedded.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require_relative '../../ext/sass/cli' +require_relative 'canonicalize_context' require_relative 'compile_error' require_relative 'compile_result' require_relative 'embedded/connection' diff --git a/lib/sass/embedded/host/importer_registry.rb b/lib/sass/embedded/host/importer_registry.rb index 18276f3e..51b39002 100644 --- a/lib/sass/embedded/host/importer_registry.rb +++ b/lib/sass/embedded/host/importer_registry.rb @@ -26,7 +26,7 @@ def initialize(importers, load_paths, alert_color:) end def register(importer) - importer = Structifier.to_struct(importer, :canonicalize, :load, :find_file_url) + importer = Structifier.to_struct(importer, :canonicalize, :load, :non_canonical_scheme, :find_file_url) is_importer = importer.respond_to?(:canonicalize) && importer.respond_to?(:load) is_file_importer = importer.respond_to?(:find_file_url) @@ -39,7 +39,17 @@ def register(importer) @importers_by_id[id] = importer if is_importer EmbeddedProtocol::InboundMessage::CompileRequest::Importer.new( - importer_id: id + importer_id: id, + non_canonical_scheme: if importer.respond_to?(:non_canonical_scheme) + non_canonical_scheme = importer.non_canonical_scheme + if non_canonical_scheme.is_a?(String) + [non_canonical_scheme] + else + non_canonical_scheme || [] + end + else + [] + end ) else EmbeddedProtocol::InboundMessage::CompileRequest::Importer.new( @@ -50,7 +60,8 @@ def register(importer) def canonicalize(canonicalize_request) importer = @importers_by_id[canonicalize_request.importer_id] - url = importer.canonicalize(canonicalize_request.url, from_import: canonicalize_request.from_import)&.to_s + url = importer.canonicalize(canonicalize_request.url, + Protofier.from_proto_canonicalize_context(canonicalize_request))&.to_s EmbeddedProtocol::InboundMessage::CanonicalizeResponse.new( id: canonicalize_request.id, @@ -84,7 +95,8 @@ def import(import_request) def file_import(file_import_request) importer = @importers_by_id[file_import_request.importer_id] - file_url = importer.find_file_url(file_import_request.url, from_import: file_import_request.from_import)&.to_s + file_url = importer.find_file_url(file_import_request.url, + Protofier.from_proto_canonicalize_context(file_import_request))&.to_s EmbeddedProtocol::InboundMessage::FileImportResponse.new( id: file_import_request.id, diff --git a/lib/sass/embedded/protofier.rb b/lib/sass/embedded/protofier.rb index 4e3a4ed6..2b6792ae 100644 --- a/lib/sass/embedded/protofier.rb +++ b/lib/sass/embedded/protofier.rb @@ -8,6 +8,13 @@ class Embedded module Protofier module_function + def from_proto_canonicalize_context(canonicalize_request) + CanonicalizeContext.new( + canonicalize_request.containing_url == '' ? nil : canonicalize_request.containing_url, + canonicalize_request.from_import + ) + end + def from_proto_compile_response(compile_response) oneof = compile_response.result result = compile_response.public_send(oneof) diff --git a/spec/sass_importer_spec.rb b/spec/sass_importer_spec.rb index 051a6a1f..1535606e 100644 --- a/spec/sass_importer_spec.rb +++ b/spec/sass_importer_spec.rb @@ -7,7 +7,7 @@ result = described_class.compile_string( '@import "orange";', importers: [{ - canonicalize: ->(url, **) { "u:#{url}" }, + canonicalize: ->(url, _context) { "u:#{url}" }, load: lambda { |url| color = url.split(':')[1] return { @@ -67,7 +67,7 @@ result = described_class.compile_string( '@import "/orange";', importers: [{ - canonicalize: lambda { |url, **| + canonicalize: lambda { |url, _context| expect(url).to eq('/orange') "u:#{url}" }, @@ -87,7 +87,7 @@ result = described_class.compile_string( '@import "C:/orange";', importers: [{ - canonicalize: lambda { |url, **| + canonicalize: lambda { |url, _context| expect(url).to eq('file:///C:/orange') "u:#{url}" }, @@ -104,11 +104,269 @@ end end + describe 'the containing URL' do + it 'is nil for a potentially canonical scheme' do + result = described_class.compile_string( + '@import "u:orange"', + importers: [{ + canonicalize: lambda { |url, context| + expect(context.containing_url).to be_nil + url + }, + load: lambda { |*| + return { + contents: '@a', + syntax: 'scss' + } + } + }] + ) + + expect(result.css).to eq('@a;') + end + + describe 'for a non-canonical scheme' do + describe 'in a list' do + it 'is set to the original URL' do + result = described_class.compile_string( + '@import "u:orange"', + importers: [{ + canonicalize: lambda { |url, context| + expect(context.containing_url).to eq('x:original.scss') + url.gsub(/^u:/, 'x:') + }, + load: lambda { |*| + return { + contents: '@a', + syntax: 'scss' + } + }, + non_canonical_scheme: ['u'] + }], + url: 'x:original.scss' + ) + + expect(result.css).to eq('@a;') + end + + it 'is nil if the original URL is nil' do + result = described_class.compile_string( + '@import "u:orange"', + importers: [{ + canonicalize: lambda { |url, context| + expect(context.containing_url).to be_nil + url.gsub(/^u:/, 'x:') + }, + load: lambda { |*| + return { + contents: '@a', + syntax: 'scss' + } + }, + non_canonical_scheme: ['u'] + }] + ) + + expect(result.css).to eq('@a;') + end + end + + describe 'as a string' do + it 'is set to the original URL' do + result = described_class.compile_string( + '@import "u:orange"', + importers: [{ + canonicalize: lambda { |url, context| + expect(context.containing_url).to eq('x:original.scss') + url.gsub(/^u:/, 'x:') + }, + load: lambda { |*| + return { + contents: '@a', + syntax: 'scss' + } + }, + non_canonical_scheme: 'u' + }], + url: 'x:original.scss' + ) + + expect(result.css).to eq('@a;') + end + + it 'is nil if the original URL is nil' do + result = described_class.compile_string( + '@import "u:orange"', + importers: [{ + canonicalize: lambda { |url, context| + expect(context.containing_url).to be_nil + url.gsub(/^u:/, 'x:') + }, + load: lambda { |*| + return { + contents: '@a', + syntax: 'scss' + } + }, + non_canonical_scheme: 'u' + }] + ) + + expect(result.css).to eq('@a;') + end + end + end + + describe 'for a schemeless load' do + it 'is set to the original URL' do + result = described_class.compile_string( + '@import "orange"', + importers: [{ + canonicalize: lambda { |url, context| + expect(context.containing_url).to eq('x:original.scss') + "u:#{url}" + }, + load: lambda { |*| + return { + contents: '@a', + syntax: 'scss' + } + } + }], + url: 'x:original.scss' + ) + + expect(result.css).to eq('@a;') + end + + it 'is nil if the original URL is nil' do + result = described_class.compile_string( + '@import "orange"', + importers: [{ + canonicalize: lambda { |url, context| + expect(context.containing_url).to be_nil + "u:#{url}" + }, + load: lambda { |*| + return { + contents: '@a', + syntax: 'scss' + } + } + }] + ) + + expect(result.css).to eq('@a;') + end + end + end + + describe 'throws an error if the importer returns a canonical URL with a non-canonical scheme' do + it 'set as a list' do + expect do + described_class.compile_string( + '@import "orange"', + importers: [{ + canonicalize: lambda { |url, _context| + "u:#{url}" + }, + load: lambda { |*| + return { + contents: '@a', + syntax: 'scss' + } + }, + non_canonical_scheme: ['u'] + }] + ) + end.to raise_sass_compile_error.with_line(0) + end + + it 'set as a string' do + expect do + described_class.compile_string( + '@import "orange"', + importers: [{ + canonicalize: lambda { |url, _context| + "u:#{url}" + }, + load: lambda { |*| + return { + contents: '@a', + syntax: 'scss' + } + }, + non_canonical_scheme: 'u' + }] + ) + end.to raise_sass_compile_error.with_line(0) + end + end + + describe 'throws an error for an invalid scheme:' do + it 'empty' do + expect do + described_class.compile_string( + 'a {b: c}', + importers: [{ + canonicalize: lambda { |*| + }, + load: lambda { |*| + return { + contents: '@a', + syntax: 'scss' + } + }, + non_canonical_scheme: '' + }] + ) + end.to raise_sass_compile_error + end + + it 'uppercase' do + expect do + described_class.compile_string( + 'a {b: c}', + importers: [{ + canonicalize: lambda { |*| + }, + load: lambda { |*| + return { + contents: '@a', + syntax: 'scss' + } + }, + non_canonical_scheme: 'U' + }] + ) + end.to raise_sass_compile_error + end + + it 'colon' do + expect do + described_class.compile_string( + 'a {b: c}', + importers: [{ + canonicalize: lambda { |*| + }, + load: lambda { |*| + return { + contents: '@a', + syntax: 'scss' + } + }, + non_canonical_scheme: 'u:' + }] + ) + end.to raise_sass_compile_error + end + end + it "uses an importer's source map URL" do result = described_class.compile_string( '@import "orange";', importers: [{ - canonicalize: lambda { |url, **| + canonicalize: lambda { |url, _context| "u:#{url}" }, load: lambda { |url| @@ -147,7 +405,7 @@ described_class.compile_string( '@import "orange";', importers: [{ - canonicalize: lambda { |url, **| + canonicalize: lambda { |url, _context| "u:#{url}" }, load: lambda { |*| @@ -184,7 +442,7 @@ described_class.compile_string( '@import "other";', importers: [{ - canonicalize: lambda { |url, **| + canonicalize: lambda { |url, _context| "u:#{url}" }, load: ->(*) {} @@ -227,7 +485,7 @@ result = described_class.compile( dir.path('input.scss'), importers: [{ - canonicalize: lambda { |url, **| + canonicalize: lambda { |url, _context| "u:#{url}" }, load: lambda { |*| @@ -305,7 +563,7 @@ result = described_class.compile_string( '@import "orange";', importer: { - canonicalize: lambda { |url, **| + canonicalize: lambda { |url, _context| expect(url).to eq('u:orange') url }, @@ -327,7 +585,7 @@ result = described_class.compile_string( '@import "other";', importer: { - canonicalize: lambda { |url, **| + canonicalize: lambda { |url, _context| url }, load: lambda { |_url| @@ -363,7 +621,7 @@ } }, importers: [{ - canonicalize: lambda { |url, **| + canonicalize: lambda { |url, _context| expect(url).to eq('u:orange') url }, @@ -393,7 +651,7 @@ } }, importers: [{ - canonicalize: lambda { |url, **| + canonicalize: lambda { |url, _context| url }, load: lambda { |url| @@ -426,7 +684,7 @@ result = described_class.compile_string( '@import "second:other";', importers: [{ - canonicalize: lambda { |url, **| + canonicalize: lambda { |url, _context| url if url.start_with?('first:') }, load: lambda { |*| @@ -436,7 +694,7 @@ } } }, { - canonicalize: lambda { |url, **| + canonicalize: lambda { |url, _context| raise 'Second importer should only be called once.' if second_called second_called = true @@ -457,8 +715,8 @@ describe 'from_import is' do def expect_from_import(canonicalize, expected) - allow(canonicalize).to receive(:call) { |url, from_import:| - expect(from_import).to be(expected) + allow(canonicalize).to receive(:call) { |url, context| + expect(context.from_import).to be(expected) "u:#{url}" } { @@ -578,7 +836,7 @@ def expect_from_import(canonicalize, expected) result = described_class.compile_string( '@import "u:other";', importers: [{ - find_file_url: lambda { |url, **| + find_file_url: lambda { |url, _context| expect(url).to eq('u:other') dir.url('dir/other') } @@ -709,8 +967,8 @@ def expect_from_import(canonicalize, expected) described_class.compile_string( '@import "other";', importers: [{ - find_file_url: lambda { |*, from_import:| - expect(from_import).to be(true) + find_file_url: lambda { |_url, context| + expect(context.from_import).to be(true) dir.url('other') } }] @@ -725,8 +983,8 @@ def expect_from_import(canonicalize, expected) described_class.compile_string( '@use "other";', importers: [{ - find_file_url: lambda { |*, from_import:| - expect(from_import).to be(false) + find_file_url: lambda { |_url, context| + expect(context.from_import).to be(false) dir.url('other') } }] @@ -736,6 +994,58 @@ def expect_from_import(canonicalize, expected) end end + describe 'containing_url is' do + it 'set for a relative URL' do + sandbox do |dir| + dir.write({ '_other.scss' => 'a {b: c}' }) + result = described_class.compile_string( + '@import "other";', + importers: [{ + find_file_url: lambda { |_url, context| + expect(context.containing_url).to eq('x:original.scss') + dir.url('other') + } + }], + url: 'x:original.scss' + ) + expect(result.css).to eq("a {\n b: c;\n}") + end + end + + it 'set for an absolute URL' do + sandbox do |dir| + dir.write({ '_other.scss' => 'a {b: c}' }) + result = described_class.compile_string( + '@import "u:other";', + importers: [{ + find_file_url: lambda { |_url, context| + expect(context.containing_url).to eq('x:original.scss') + dir.url('other') + } + }], + url: 'x:original.scss' + ) + expect(result.css).to eq("a {\n b: c;\n}") + end + end + + it 'unset when the URL is unavailable' do + sandbox do |dir| + dir.write({ '_other.scss' => 'a {b: c}' }) + result = described_class.compile_string( + '@import "u:other";', + importers: [{ + find_file_url: lambda { |_url, context| + expect(context.containing_url).to be_nil + dir.url('other') + } + }] + ) + expect(result.css).to eq("a {\n b: c;\n}") + end + end + end + it "throws an error for an importer that's ambiguous between FileImporter and Importer" do sandbox do |dir| dir.write({ '_other.scss' => 'a {b: c}' }) @@ -765,7 +1075,7 @@ def expect_from_import(canonicalize, expected) described_class.compile_string( '@import "other";', importers: [{ - canonicalize: ->(url, **) { "u:#{url}" }, + canonicalize: ->(url, _context) { "u:#{url}" }, load: lambda { |*| return { contents: StringIO.new('not a string'), @@ -785,7 +1095,7 @@ def expect_from_import(canonicalize, expected) described_class.compile_string( '@import "other";', importers: [{ - canonicalize: ->(url, **) { "u:#{url}" }, + canonicalize: ->(url, _context) { "u:#{url}" }, load: lambda { |*| return { contents: '',