Skip to content

[BUGFIX] Fix SCSS compilation, cache handling and import resolution - #103

Open
davidk1982 wants to merge 1 commit into
WapplerSystems:release/v13from
davidk1982:bugfix/fix-scss-compilation-v13
Open

davidk1982 wants to merge 1 commit into
WapplerSystems:release/v13from
davidk1982:bugfix/fix-scss-compilation-v13

Conversation

@davidk1982

Copy link
Copy Markdown

Summary

Fixes SCSS compilation that was broken due to incompatibilities with scssphp v2 (Dart Sass).
Removes the complex Importer system in favor of a simplified, working approach using
compileString() with variable injection as SCSS source code.

Changes

Compiler.php — Complete rewrite

  • Variable injection via SCSS declarations instead of addVariables() with Value objects.
    scssphp v2 requires Value objects for addVariables(), which cannot handle SCSS expressions
    like $line-height-base - .25. Injecting as SCSS source code evaluates them in context.
  • Two-phase variable injection: Literals/internal expressions prepended before import
    (override !default), external $var references appended after import (defensive fallback).
  • Path value quoting: Values starting with / are auto-quoted to prevent parse errors
    (e.g. /_assets/... paths from EXT: resolution would be interpreted as division).
  • compileFile() → compileString(): Required for source-level variable injection.
  • Import paths: dirname-based, EXT: callable resolver, and vendor directory path.
  • Cache key fix: Includes cssFilePath so different output targets get separate cache entries.
  • File existence check: Verifies CSS file exists before returning cached path.

RenderPreProcessorHook.php

  • Added declare(strict_types=1) and typed properties
  • v13-compatible TypoScript access via $GLOBALS['TYPO3_REQUEST']->getAttribute('frontend.typoscript')
  • Added EXT: variable resolution (resolves EXT:ext/... → /_assets/{hash}/... web paths)
  • Fixed parseBooleanSetting() null handling
  • Added outputfile path handling (compress=false + path normalization)

Removed (unused/broken with scssphp v2)

  • Classes/ImportResolver.php
  • Classes/Importer/ExtensionFilesystemImporter.php
  • Classes/Importer/FilesystemImporter.php
  • Classes/Importer/VariableFilesystemImporter.php

Technical Context

scssphp v2 (Dart Sass compliant) introduced breaking changes:

  • addVariables() requires Value objects — raw strings no longer accepted
  • Compiler constructor takes no parameters (no cache options)
  • Strict unit math: unitless - em operations are invalid
  • Variable evaluation is eager (at declaration time, not lazy)

The Importer-based approach from the original v13 code did not correctly handle
TypoScript variable expressions and produced incorrect CSS output (literal $variable
strings in CSS).

Note on v14 compatibility

The release/v14 branch (14.0.4) has the same underlying bugs in its variable handling:

  • ValueConverter::fromPhp() produces quoted strings for SCSS expressions
  • SCSS expressions like $line-height-base - .25 are not evaluated, output as literal strings
  • Cache key does not include output file path

The approach used in this PR (SCSS source injection + two-phase ordering) is directly
applicable to v14 as well. A separate PR for release/v14 could port this fix with
minimal adaptation (mainly the ScssViewHelper refactoring and composer constraints).

Testing

Tested with Bootstrap 4 SCSS compilation via theme extensions using:

  • Literal color values, rem/px units
  • SCSS expressions referencing other TypoScript variables ($font-size-base * 1.875)
  • EXT: path resolution for font/icon variables
  • Multiple output targets (outputfile TypoScript option)
  • Source maps (inline)

- Simplify Compiler to use scssphp addVariables() with string values
  instead of complex Value type conversion (SassColor, SassNumber, etc.)
- Use compileString('@import "...";') for reliable import resolution
  instead of manually reading files and custom Importer classes
- Fix cache key to include cssFilePath so different outputfile targets
  (e.g. theme.css vs custom-theme.css) get separate cache entries
- Move cache check before compiler setup for early return (performance)
- Add vendor directory as import path so SCSS files can use
  @import "vendor-name/package-name/..." without fragile relative paths
- Add vendor path fallback in calculateContentHash for hash accuracy
- Add EXT: variable resolution in RenderPreProcessorHook to convert
  EXT: prefixed TypoScript variable values to web-accessible paths
- Add absolute path comparison for includeCSS file matching
- Fix parseBooleanSetting() strict type error with nullable defaults
- Add typed properties and declare(strict_types=1)
- Remove unused ImportResolver and Importer classes
- Replace debug() with DebugUtility::debug()

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

1 participant