Reject imports Stylesheets::bundle() cannot inline faithfully
Plan step 37 review. bundle() left an @import it did not recognise exactly as written, in the middle of the output, where a browser ignores it: a layer(), supports() or media condition was silently dropped, an unquoted url(./x.css) import was left in place with its path mangled by the url() rewrite, and a bare specifier or absolute URL vanished the same way. An @import after a rule or inside a @layer block was inlined anyway, nesting a whole file's layers inside another. Each now throws, naming the file; unquoted url() imports and @IMPORT inline like the other forms. The cache keyed only the top-level files' mtimes, so a changed button.css left a cached all.css bundle standing in a long-lived worker; a cached bundle is now served only while every file it inlined keeps its mtime. A null and an empty $base no longer share a cache key, a leading @charset or byte-order mark is dropped from each inlined file, and a test pins the import graph free of cycles, the one case where a depth-first bundle would place a stylesheet before a file it imports. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qwx5USif3wFFmxtHg5U1g9
This commit is contained in:
co-authored by
Claude Opus 5
parent
792ec44d5e
commit
3beeb2fc22
+97
-70
@@ -15,26 +15,39 @@ use RuntimeException;
|
||||
* `bundle()` walks each given file's `@import`s depth-first, in source order, and replaces each
|
||||
* one with the imported file's own content (which is itself walked the same way) the first time
|
||||
* that file is reached; a later `@import` of the same file, anywhere in the graph, is simply
|
||||
* dropped, since the file's rules are already in the output at their first position. Only a
|
||||
* relative import — `@import './x.css';` or `@import '../x.css';`, with or without `url()` — is
|
||||
* resolved this way; a bare specifier (`@import 'tailwindcss';`) or an absolute URL is left
|
||||
* exactly as written, the way postcss-import treats the first as a package to resolve elsewhere
|
||||
* and the second as a network resource. The `@layer` statement every package stylesheet opens
|
||||
* with is ordinary content here: it is not deduplicated or hoisted, because a repeated `@layer`
|
||||
* statement naming the same sub-layers in the same order is valid anywhere at the top level and
|
||||
* changes nothing — the output keeps one copy per file it inlines.
|
||||
* dropped, since the file's rules are already in the output at their first position. A file is
|
||||
* one file however it is reached (`./a.css` or `../components/a.css`): identity is its real path.
|
||||
* Because every package stylesheet imports what it depends on before its own rules, a file's
|
||||
* rules always follow those of every file it imports, so an override that ties on specificity
|
||||
* still lands after the rule it overrides.
|
||||
*
|
||||
* A `url()` this class finds — a font, an SVG mask — is only ever relative in the tree this class
|
||||
* ships with, so only a relative one is rewritten (an absolute URL, a `data:` URI or a `#`
|
||||
* fragment is left alone): resolved against the file that wrote it, then re-expressed relative to
|
||||
* the directory of `$files[0]` — `resources/css/` for the package's own entry points, so without
|
||||
* `$base` a font or an SVG stays reachable exactly as it would from a file sitting there. `$base`
|
||||
* relocates that path instead, onto the directory or URL the bundle is actually served from.
|
||||
* Only the form every package stylesheet writes is accepted — `@import './x.css';` or
|
||||
* `@import url('./x.css');`, before any rule. Anything the output could not keep meaning the same
|
||||
* throws, naming the file: a `layer()`, `supports()` or media condition (inlining would drop it),
|
||||
* an absolute URL or path (a browser would ignore an `@import` in the middle of the bundle), an
|
||||
* `@import` after a rule or inside a block (CSS ignores the first; the second would nest a whole
|
||||
* file's layers inside another), or a target that does not exist — which is also what a package
|
||||
* specifier such as `tailwindcss` is to a resolver that knows no `node_modules`. A leading
|
||||
* `@charset` (and a byte-order mark) is dropped from every file, since it means nothing past the
|
||||
* first byte of a stylesheet; the bundle is UTF-8. The `@layer` statement every package
|
||||
* stylesheet opens with is ordinary content: a repeated `@layer` statement naming the same
|
||||
* sub-layers in the same order is valid anywhere at the top level and changes nothing.
|
||||
*
|
||||
* A `url()` outside comments and strings — a font, an SVG mask — is rewritten when it is relative
|
||||
* (an absolute URL or path, a `data:` URI or a `#` fragment is left alone): resolved against the
|
||||
* file that wrote it, then re-expressed relative to the directory of `$files[0]` —
|
||||
* `resources/css/` for the package's own entry points, so without `$base` a font or an SVG stays
|
||||
* reachable exactly as it would from a file sitting there. `$base` is the directory or URL that
|
||||
* directory is served as, and every relative `url()` is joined onto it.
|
||||
*
|
||||
* Bundles are cached for the life of the process, per file list and `$base`, and a cached bundle
|
||||
* is served only while every file it inlined — nested imports included — keeps its modification
|
||||
* time.
|
||||
*/
|
||||
final class Stylesheets
|
||||
{
|
||||
/**
|
||||
* @var array<string, string>
|
||||
* @var array<string, array{css: string, files: array<string, int|false>}>
|
||||
*/
|
||||
private static array $cache = [];
|
||||
|
||||
@@ -51,21 +64,23 @@ final class Stylesheets
|
||||
return '';
|
||||
}
|
||||
|
||||
$key = self::cacheKey($files, $base);
|
||||
$key = serialize([$files, $base]);
|
||||
|
||||
if (array_key_exists($key, self::$cache)) {
|
||||
return self::$cache[$key];
|
||||
if (isset(self::$cache[$key]) && self::fresh(self::$cache[$key]['files'])) {
|
||||
return self::$cache[$key]['css'];
|
||||
}
|
||||
|
||||
$root = dirname(self::resolve($files[0], null));
|
||||
$root = dirname(self::resolve($files[0]));
|
||||
$seen = [];
|
||||
$parts = [];
|
||||
$css = '';
|
||||
|
||||
foreach ($files as $file) {
|
||||
$parts[] = self::inline(self::resolve($file, null), $seen, [], $root, $base);
|
||||
$css .= self::inline(self::resolve($file), $seen, $root, $base);
|
||||
}
|
||||
|
||||
return self::$cache[$key] = implode('', $parts);
|
||||
self::$cache[$key] = ['css' => $css, 'files' => $seen];
|
||||
|
||||
return $css;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -78,47 +93,51 @@ final class Stylesheets
|
||||
}
|
||||
|
||||
/**
|
||||
* @param list<string> $files
|
||||
* @param array<string, int|false> $files every file a cached bundle inlined, with its mtime then
|
||||
*/
|
||||
private static function cacheKey(array $files, ?string $base): string
|
||||
private static function fresh(array $files): bool
|
||||
{
|
||||
$parts = array_map(function (string $file): string {
|
||||
$real = realpath($file);
|
||||
foreach ($files as $file => $mtime) {
|
||||
clearstatcache(true, $file);
|
||||
|
||||
return $real === false ? $file : $real.'@'.filemtime($real);
|
||||
}, $files);
|
||||
if ((is_file($file) ? filemtime($file) : false) !== $mtime) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
return implode('|', $parts).'#'.($base ?? '');
|
||||
return true;
|
||||
}
|
||||
|
||||
private static function resolve(string $path, ?string $importer): string
|
||||
/**
|
||||
* `$path`'s real path; `$importer` and `$target`, when given, name the stylesheet and the
|
||||
* `@import` target as written, for the exception.
|
||||
*/
|
||||
private static function resolve(string $path, ?string $importer = null, ?string $target = null): string
|
||||
{
|
||||
$real = realpath($path);
|
||||
|
||||
if ($real === false) {
|
||||
if ($real === false || ! is_file($real)) {
|
||||
throw new RuntimeException($importer === null
|
||||
? "Stylesheets::bundle() cannot read \"{$path}\": it does not exist."
|
||||
: "{$importer} imports \"{$path}\", which does not exist.");
|
||||
: "{$importer} imports \"{$target}\", which does not exist.");
|
||||
}
|
||||
|
||||
return $real;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param array<string, true> $seen every file already inlined, by resolved path
|
||||
* @param list<string> $stack the files currently being inlined, to break a cycle
|
||||
* @param array<string, int|false> $seen every file already inlined, by real path, with its mtime
|
||||
*/
|
||||
private static function inline(string $file, array &$seen, array $stack, string $root, ?string $base): string
|
||||
private static function inline(string $file, array &$seen, string $root, ?string $base): string
|
||||
{
|
||||
if (isset($seen[$file]) || in_array($file, $stack, true)) {
|
||||
if (array_key_exists($file, $seen)) {
|
||||
return '';
|
||||
}
|
||||
|
||||
$seen[$file] = true;
|
||||
$stack[] = $file;
|
||||
// Marked before its imports are walked, so a cycle back to this file ends there.
|
||||
$seen[$file] = filemtime($file);
|
||||
|
||||
$css = (string) file_get_contents($file);
|
||||
$masked = self::mask($css);
|
||||
$css = (string) preg_replace('/\A(?:\xEF\xBB\xBF)?@charset\s+"[^"]*"\s*;/i', '', (string) file_get_contents($file));
|
||||
$dir = dirname($file);
|
||||
$result = '';
|
||||
$cursor = 0;
|
||||
@@ -127,13 +146,9 @@ final class Stylesheets
|
||||
// below, so only this file's own text — never the spliced-in content of another file —
|
||||
// is passed to rewriteUrls() here; running it again over the concatenated result would
|
||||
// resolve an already-rewritten url() a second time, against the wrong directory.
|
||||
foreach (self::imports($css, $masked) as $import) {
|
||||
foreach (self::imports($file, $css) as $import) {
|
||||
$result .= self::rewriteUrls(substr($css, $cursor, $import['start'] - $cursor), $dir, $root, $base);
|
||||
|
||||
$result .= self::isRelativeImport($import['target'])
|
||||
? self::inline(self::resolve($dir.'/'.$import['target'], self::name($file)), $seen, $stack, $root, $base)
|
||||
: substr($css, $import['start'], $import['end'] - $import['start']);
|
||||
|
||||
$result .= self::inline(self::resolve($dir.'/'.$import['target'], self::name($file), $import['target']), $seen, $root, $base);
|
||||
$cursor = $import['end'];
|
||||
}
|
||||
|
||||
@@ -147,7 +162,7 @@ final class Stylesheets
|
||||
*/
|
||||
private static function name(string $file): string
|
||||
{
|
||||
$package = dirname(__DIR__, 2).'/resources/css/';
|
||||
$package = realpath(dirname(__DIR__, 2).'/resources/css').'/';
|
||||
|
||||
return str_starts_with($file, $package) ? substr($file, strlen($package)) : $file;
|
||||
}
|
||||
@@ -206,52 +221,63 @@ final class Stylesheets
|
||||
}
|
||||
|
||||
/**
|
||||
* Every top-level `@import` statement in `$css`, found through `$masked` so one inside a
|
||||
* comment or a string never counts, each with the byte range of the whole statement (the
|
||||
* semicolon included) and the quoted target it names.
|
||||
* Every `@import` statement in `$css` outside comments and strings, each with the byte range of
|
||||
* the whole statement (the semicolon included) and the target it names — or an exception, for
|
||||
* one the bundle could not inline without changing what it means (see the class comment).
|
||||
*
|
||||
* @return list<array{start: int, end: int, target: string}>
|
||||
*/
|
||||
private static function imports(string $css, string $masked): array
|
||||
private static function imports(string $file, string $css): array
|
||||
{
|
||||
$masked = self::mask($css);
|
||||
$imports = [];
|
||||
|
||||
if (preg_match_all('/@import\b/', $masked, $matches, PREG_OFFSET_CAPTURE) === 0) {
|
||||
if (preg_match_all('/@import\b/i', $masked, $matches, PREG_OFFSET_CAPTURE) === 0) {
|
||||
return $imports;
|
||||
}
|
||||
|
||||
// Where the text allowed before the next @import starts: only whitespace, blinded comments,
|
||||
// `@charset` and `@layer` statements and earlier @imports may precede one.
|
||||
$allowedFrom = 0;
|
||||
|
||||
foreach ($matches[0] as [, $start]) {
|
||||
$end = strpos($masked, ';', $start);
|
||||
$end = $end === false ? strlen($css) : $end + 1;
|
||||
$statement = substr($css, $start, $end - $start);
|
||||
$statement = trim(substr($css, $start, $end - $start));
|
||||
|
||||
if (preg_match('/^@import\s+(?:url\(\s*)?([\'"])(.*?)\1\)?\s*;?\s*$/s', trim($statement), $match) === 1) {
|
||||
$imports[] = ['start' => $start, 'end' => $end, 'target' => $match[2]];
|
||||
$before = (string) preg_replace('/@(?:layer|charset)\s[^;{}]*;/i', '', substr($masked, $allowedFrom, $start - $allowedFrom));
|
||||
|
||||
if (trim($before) !== '') {
|
||||
throw new RuntimeException(self::name($file)." writes `{$statement}` after a rule or inside a block; Stylesheets::bundle() inlines an @import only before every rule, where CSS reads it.");
|
||||
}
|
||||
|
||||
if (preg_match('/^@import\s*(?:url\(\s*([\'"]?)([^\'"()\s]+)\1\s*\)|([\'"])(.+?)\3)\s*;?$/is', $statement, $match, PREG_UNMATCHED_AS_NULL) !== 1) {
|
||||
throw new RuntimeException(self::name($file)." writes `{$statement}`; Stylesheets::bundle() inlines a plain @import only, without layer(), supports() or a media condition, which the bundle would drop.");
|
||||
}
|
||||
|
||||
$target = $match[2] ?? $match[4] ?? '';
|
||||
|
||||
if (preg_match('~^(?:[a-z][a-z0-9+.-]*:|/)~i', $target) === 1) {
|
||||
throw new RuntimeException(self::name($file)." imports \"{$target}\", an absolute URL or path; Stylesheets::bundle() inlines files relative to the importing stylesheet only.");
|
||||
}
|
||||
|
||||
$imports[] = ['start' => $start, 'end' => $end, 'target' => $target];
|
||||
$allowedFrom = $end;
|
||||
}
|
||||
|
||||
return $imports;
|
||||
}
|
||||
|
||||
/**
|
||||
* A bare specifier (`tailwindcss`) or an absolute URL is not resolved against the importing
|
||||
* file — only `./x` and `../x` are, the two forms every package stylesheet writes.
|
||||
*/
|
||||
private static function isRelativeImport(string $target): bool
|
||||
{
|
||||
return str_starts_with($target, './') || str_starts_with($target, '../');
|
||||
}
|
||||
|
||||
/**
|
||||
* Every relative `url()` in `$css` re-expressed against `$root` (or `$base`, once joined onto
|
||||
* it below), resolved first against `$dir`, the directory of the file that wrote it. An
|
||||
* absolute URL, a `data:` URI or a `#` fragment is left untouched.
|
||||
* absolute URL or path, a `data:` URI or a `#` fragment is left untouched.
|
||||
*/
|
||||
private static function rewriteUrls(string $css, string $dir, string $root, ?string $base): string
|
||||
{
|
||||
$masked = self::mask($css);
|
||||
|
||||
if (preg_match_all('/url\(/', $masked, $matches, PREG_OFFSET_CAPTURE) === 0) {
|
||||
if (preg_match_all('/url\(/i', $masked, $matches, PREG_OFFSET_CAPTURE) === 0) {
|
||||
return $css;
|
||||
}
|
||||
|
||||
@@ -271,7 +297,7 @@ final class Stylesheets
|
||||
|
||||
$call = substr($css, $start, $close - $start + 1);
|
||||
|
||||
if (preg_match('/^url\(\s*([\'"]?)(.*?)\1\s*\)$/s', $call, $match) !== 1 || ! self::isRelativeUrl($match[2])) {
|
||||
if (preg_match('/^url\(\s*([\'"]?)(.*?)\1\s*\)$/is', $call, $match) !== 1 || ! self::isRelativeUrl($match[2])) {
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -296,8 +322,9 @@ final class Stylesheets
|
||||
}
|
||||
|
||||
/**
|
||||
* `$path`, its `.` and `..` segments resolved away. Pure string handling — the target need not
|
||||
* exist on disk, since a bundle a test builds may not ship the fonts and SVGs it points at.
|
||||
* `$path`, its `.` and `..` segments resolved away (a `..` at the root of an absolute path
|
||||
* stays at the root). Pure string handling — the target need not exist on disk, since a bundle
|
||||
* a test builds may not ship the fonts and SVGs it points at.
|
||||
*/
|
||||
private static function normalise(string $path): string
|
||||
{
|
||||
@@ -311,7 +338,7 @@ final class Stylesheets
|
||||
|
||||
if ($segment === '..' && $segments !== [] && end($segments) !== '..') {
|
||||
array_pop($segments);
|
||||
} else {
|
||||
} elseif ($segment !== '..' || ! $absolute) {
|
||||
$segments[] = $segment;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -102,6 +102,68 @@ it('keeps a file two others import once, at its first position', function () {
|
||||
}
|
||||
});
|
||||
|
||||
it('inlines every plain import form once, and nothing written in a comment or a string', function () {
|
||||
$dir = sys_get_temp_dir().'/livewire-material-bundle-'.Str::random(8);
|
||||
File::makeDirectory("{$dir}/components", recursive: true);
|
||||
|
||||
foreach (['double', 'quoted-url', 'bare-url'] as $name) {
|
||||
// A leading byte-order mark and @charset mean nothing past a stylesheet's first byte.
|
||||
File::put("{$dir}/components/{$name}.css", "\u{FEFF}@charset \"UTF-8\";\n.{$name} {}\n");
|
||||
}
|
||||
|
||||
File::put("{$dir}/entry.css", <<<'CSS'
|
||||
/* An example in a header: @import './components/never.css'; url(never.png) — it's fine. */
|
||||
@layer a, b;
|
||||
@import "./components/double.css";
|
||||
@import url('./components/quoted-url.css');
|
||||
@IMPORT url( ./components/bare-url.css );
|
||||
@import './components/../components/double.css';
|
||||
.entry::before { content: "@import './components/never.css'; url(never.png)"; }
|
||||
CSS);
|
||||
|
||||
try {
|
||||
$bundled = Stylesheets::bundle(["{$dir}/entry.css"]);
|
||||
|
||||
expect(substr_count($bundled, '.double {}'))->toBe(1)
|
||||
->and($bundled)->toContain('.quoted-url {}')
|
||||
->toContain('.bare-url {}')
|
||||
->toContain("/* An example in a header: @import './components/never.css'; url(never.png) — it's fine. */")
|
||||
->toContain(".entry::before { content: \"@import './components/never.css'; url(never.png)\"; }")
|
||||
->not->toContain('@charset')
|
||||
->not->toContain("\u{FEFF}")
|
||||
// The two left are the comment's and the string's.
|
||||
->and(substr_count(strtolower($bundled), '@import'))->toBe(2);
|
||||
} finally {
|
||||
File::deleteDirectory($dir);
|
||||
}
|
||||
});
|
||||
|
||||
it('throws, naming the file, for an import the bundle could not inline without changing its meaning', function (string $css, string $reason) {
|
||||
$dir = sys_get_temp_dir().'/livewire-material-bundle-'.Str::random(8);
|
||||
File::makeDirectory($dir);
|
||||
File::put("{$dir}/x.css", '.x {}');
|
||||
File::put("{$dir}/entry.css", $css);
|
||||
|
||||
try {
|
||||
Stylesheets::bundle(["{$dir}/entry.css"]);
|
||||
|
||||
$this->fail("Stylesheets::bundle() should have thrown for {$css}");
|
||||
} catch (RuntimeException $exception) {
|
||||
expect($exception->getMessage())->toContain('entry.css')->toContain($reason);
|
||||
} finally {
|
||||
File::deleteDirectory($dir);
|
||||
}
|
||||
})->with([
|
||||
'a layer() condition' => ["@import './x.css' layer(x);", 'without layer()'],
|
||||
'a supports() condition' => ["@import './x.css' supports(display: grid);", 'without layer()'],
|
||||
'a media condition' => ['@import url(./x.css) screen;', 'without layer()'],
|
||||
'an absolute URL' => ["@import 'https://example.com/x.css';", 'absolute URL'],
|
||||
'an absolute path' => ["@import '/x.css';", 'absolute URL'],
|
||||
'a package specifier' => ["@import 'tailwindcss';", '"tailwindcss", which does not exist'],
|
||||
'an import after a rule' => [".a {}\n@import './x.css';", 'after a rule'],
|
||||
'an import inside a block' => ["@layer b { @import './x.css'; }", 'inside a block'],
|
||||
]);
|
||||
|
||||
it('rewrites a relative url() against the entry file\'s directory, or against $base when given', function () {
|
||||
// Mirrors resources/css/'s own shape: an entry (all.css) sitting where the bundle is "from",
|
||||
// a component one level under it (components/menu.css), and an asset one level above that
|
||||
@@ -145,6 +207,32 @@ it('rewrites a relative url() against the entry file\'s directory, or against $b
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* Deduplication keeps a file at its first position, so an override that ties a rule it imports on
|
||||
* specificity (the navigation rail's header FAB over fab.css was one) still wins only while every
|
||||
* stylesheet's rules land after those of each file it imports. A depth-first inline guarantees
|
||||
* that for every import edge — except around a cycle, where one file of the two must come first.
|
||||
* So no package stylesheet may reach itself through its imports.
|
||||
*/
|
||||
it('keeps the package\'s import graph free of cycles, so a stylesheet always lands after what it imports', function () {
|
||||
$edges = 0;
|
||||
|
||||
$walk = function (string $file, array $stack) use (&$walk, &$edges): void {
|
||||
expect(in_array($file, $stack, true))->toBeFalse('import cycle: '.implode(' → ', array_map(basename(...), [...$stack, $file])));
|
||||
|
||||
preg_match_all("/@import\\s+'([^']+)'/", stylesheetsBundleWithoutComments(File::get($file)), $matches);
|
||||
|
||||
foreach ($matches[1] as $import) {
|
||||
$edges++;
|
||||
$walk((string) realpath(dirname($file).'/'.$import), [...$stack, $file]);
|
||||
}
|
||||
};
|
||||
|
||||
$walk(stylesheetsBundlePath('all.css'), []);
|
||||
|
||||
expect($edges)->toBeGreaterThan(100);
|
||||
});
|
||||
|
||||
it('does not loop on a cycle, and keeps both files\' rules', function () {
|
||||
$dir = sys_get_temp_dir().'/livewire-material-bundle-'.Str::random(8);
|
||||
File::makeDirectory($dir);
|
||||
@@ -206,6 +294,20 @@ it('caches per file list and mtime, and resetCache() forces a fresh read', funct
|
||||
|
||||
expect(Stylesheets::bundle(["{$dir}/cached.css"]))
|
||||
->toBe('.after {}', 'a changed mtime should invalidate the cache on its own');
|
||||
|
||||
// So does a later mtime on a file the bundle reached only through an @import — a changed
|
||||
// button.css must not leave a cached all.css bundle standing.
|
||||
File::put("{$dir}/entry.css", "@import './cached.css';\n");
|
||||
touch("{$dir}/entry.css", 1_700_000_000);
|
||||
|
||||
expect(Stylesheets::bundle(["{$dir}/entry.css"]))->toContain('.after {}');
|
||||
|
||||
File::put("{$dir}/cached.css", '.nested-change {}');
|
||||
touch("{$dir}/cached.css", 1_700_000_200);
|
||||
|
||||
expect(Stylesheets::bundle(["{$dir}/entry.css"]))
|
||||
->toContain('.nested-change {}')
|
||||
->and(Stylesheets::bundle(["{$dir}/entry.css"], ''))->toContain('.nested-change {}');
|
||||
} finally {
|
||||
Stylesheets::resetCache();
|
||||
File::deleteDirectory($dir);
|
||||
|
||||
Reference in New Issue
Block a user