Skip to content

feat: Import parsing through an ImportParser#803

Draft
Cerdic wants to merge 2 commits intoscssphp:mainfrom
Cerdic:import_parser_interface
Draft

feat: Import parsing through an ImportParser#803
Cerdic wants to merge 2 commits intoscssphp:mainfrom
Cerdic:import_parser_interface

Conversation

@Cerdic
Copy link
Collaborator

@Cerdic Cerdic commented Aug 19, 2025

The ImportParser can be changed in the compiler with the setImportParser method before compilation.
This allow to implement an alternative ImportParser that can integrate a cache on parsed files.

See https://git.spip.net/spip-contrib-extensions/scssphp/-/merge_requests/20 for an example of usage.

This proposal follows #800 and the discussion there

…the compiler before compilation for caching purpose
Copy link
Member

@stof stof left a comment

Choose a reason for hiding this comment

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

As discussed in #800, I don't plan to merge this for now. I'd like to see how the Sass team will design the API for shared state between compilations before introducing public APIs that will be hard to refactor.

/**
* @throws SassFormatException when parsing fails
*/
public static function parse(string $contents, Syntax $syntax, ?LoggerInterface $logger = null, ?UriInterface $sourceUrl = null): Stylesheet;
Copy link
Member

Choose a reason for hiding this comment

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

I would not make it a static method, as this prevents making configurable implementations.

There is no reason to make it static as you inject an instance of the ImportParserInterface anyway, so there is no need to call it statically.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

/**
* @throws SassFormatException when parsing fails
*/
public static function parse(string $contents, Syntax $syntax, ?LoggerInterface $logger = null, ?UriInterface $sourceUrl = null): Stylesheet
Copy link
Member

Choose a reason for hiding this comment

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

I have an in-progress branch synchronizing some dart-sass changes in which the logger will disappear from the signature of Stylesheet::parse (which is totally fine as it is marked as internal and so not covered by semver), but which would break this proposal as the whole point of this ImportParser is that it is a public extension point.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we'll see then, for now the PR is on hold as you said

@stof stof marked this pull request as draft August 19, 2025 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants