Skip to content

CSS fixes #16514

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Jarred-Sumner merged 3 commits into from
Jan 19, 2025
Merged

CSS fixes #16514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3af1636
fix a lot of tests
zackradisic Jan 19, 2025
d9b072b
add media tests
zackradisic Jan 19, 2025
2057ea9
yoops
zackradisic Jan 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 39 additions & 27 deletions src/bundler/bundle_v2.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9683,18 +9683,21 @@ pub const LinkerContext = struct {
.minify = c.options.minify_whitespace,
.targets = bun.css.Targets.forBundlerTarget(c.options.target),
};
_ = css.toCssWithWriter(
_ = switch (css.toCssWithWriter(
worker.allocator,
&buffer_writer,
printer_options,
&css_import.condition_import_records,
) catch |e| {
return CompileResult{
.css = .{
.result = .{ .err = e },
.source_index = Index.invalid.get(),
},
};
)) {
.result => {},
.err => {
return CompileResult{
.css = .{
.result = .{ .err = error.PrintError },
.source_index = Index.invalid.get(),
},
};
},
};
return CompileResult{
.css = .{
Expand All @@ -9710,18 +9713,21 @@ pub const LinkerContext = struct {
.minify = c.options.minify_whitespace,
.targets = bun.css.Targets.forBundlerTarget(c.options.target),
};
_ = css.toCssWithWriter(
_ = switch (css.toCssWithWriter(
worker.allocator,
&buffer_writer,
printer_options,
&import_records,
) catch |e| {
return CompileResult{
.css = .{
.result = .{ .err = e },
.source_index = Index.invalid.get(),
},
};
)) {
.result => {},
.err => {
return CompileResult{
.css = .{
.result = .{ .err = error.PrintError },
.source_index = Index.invalid.get(),
},
};
},
};
return CompileResult{
.css = .{
Expand All @@ -9737,18 +9743,21 @@ pub const LinkerContext = struct {
// TODO: make this more configurable
.minify = c.options.minify_whitespace or c.options.minify_syntax or c.options.minify_identifiers,
};
_ = css.toCssWithWriter(
_ = switch (css.toCssWithWriter(
worker.allocator,
&buffer_writer,
printer_options,
&c.graph.ast.items(.import_records)[idx.get()],
) catch |e| {
return CompileResult{
.css = .{
.result = .{ .err = e },
.source_index = idx.get(),
},
};
)) {
.result => {},
.err => {
return CompileResult{
.css = .{
.result = .{ .err = error.PrintError },
.source_index = idx.get(),
},
};
},
};
return CompileResult{
.css = .{
Expand Down Expand Up @@ -9929,9 +9938,12 @@ pub const LinkerContext = struct {
.minify = c.options.minify_whitespace or c.options.minify_syntax or c.options.minify_identifiers,
};

const print_result = ast_import.toCss(allocator, printer_options, &entry.condition_import_records) catch |e| {
c.log.addErrorFmt(null, Loc.Empty, c.allocator, "Error generating CSS for import: {s}", .{@errorName(e)}) catch bun.outOfMemory();
continue;
const print_result = switch (ast_import.toCss(allocator, printer_options, &entry.condition_import_records)) {
.result => |v| v,
.err => |e| {
c.log.addErrorFmt(null, Loc.Empty, c.allocator, "Error generating CSS for import: {}", .{e}) catch bun.outOfMemory();
continue;
},
};
p.* = bun.fs.Path.init(DataURL.encodeStringAsShortestDataURL(allocator, "text/css", std.mem.trim(u8, print_result.code, " \n\r\t")));
}
Expand Down
99 changes: 85 additions & 14 deletions src/css/css_internals.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,42 @@ const TestKind = enum {
prefix,
};

const TestCategory = enum {
/// arg is browsers
normal,
/// arg is parser options
parser_options,
};

pub fn minifyErrorTestWithOptions(globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) bun.JSError!JSC.JSValue {
return testingImpl(globalThis, callframe, .minify, .parser_options);
}

pub fn minifyTestWithOptions(globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) bun.JSError!JSC.JSValue {
return testingImpl(globalThis, callframe, .minify);
return testingImpl(globalThis, callframe, .minify, .parser_options);
}

pub fn prefixTestWithOptions(globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) bun.JSError!JSC.JSValue {
return testingImpl(globalThis, callframe, .prefix);
return testingImpl(globalThis, callframe, .prefix, .parser_options);
}

pub fn testWithOptions(globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) bun.JSError!JSC.JSValue {
return testingImpl(globalThis, callframe, .normal);
return testingImpl(globalThis, callframe, .normal, .parser_options);
}

pub fn minifyTest(globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) bun.JSError!JSC.JSValue {
return testingImpl(globalThis, callframe, .minify, .normal);
}

pub fn testingImpl(globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame, comptime test_kind: TestKind) bun.JSError!JSC.JSValue {
pub fn prefixTest(globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) bun.JSError!JSC.JSValue {
return testingImpl(globalThis, callframe, .prefix, .normal);
}

pub fn _test(globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) bun.JSError!JSC.JSValue {
return testingImpl(globalThis, callframe, .normal, .normal);
}

pub fn testingImpl(globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame, comptime test_kind: TestKind, comptime test_category: TestCategory) bun.JSError!JSC.JSValue {
var arena = arena_ orelse brk: {
break :brk Arena.init() catch @panic("oopsie arena no good");
};
Expand Down Expand Up @@ -67,13 +90,24 @@ pub fn testingImpl(globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame, c

var browsers: ?bun.css.targets.Browsers = null;
const parser_options = parser_options: {
const opts = bun.css.ParserOptions.default(alloc, &log);
var opts = bun.css.ParserOptions.default(alloc, &log);
// if (test_kind == .prefix) break :parser_options opts;

if (browser_options_arg) |optargs| {
if (optargs.isObject()) {
browsers = try targetsFromJS(globalThis, optargs);
}
switch (test_category) {
.normal => {
if (browser_options_arg) |optargs| {
if (optargs.isObject()) {
browsers = try targetsFromJS(globalThis, optargs);
}
}
},
.parser_options => {
if (browser_options_arg) |optargs| {
if (optargs.isObject()) {
try parserOptionsFromJS(globalThis, alloc, &opts, optargs);
}
}
},
}

break :parser_options opts;
Expand All @@ -92,7 +126,7 @@ pub fn testingImpl(globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame, c
minify_options.targets.browsers = browsers;
_ = stylesheet.minify(alloc, minify_options).assert();

const result = stylesheet.toCss(alloc, bun.css.PrinterOptions{
const result = switch (stylesheet.toCss(alloc, bun.css.PrinterOptions{
.minify = switch (test_kind) {
.minify => true,
.normal => false,
Expand All @@ -101,22 +135,59 @@ pub fn testingImpl(globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame, c
.targets = .{
.browsers = minify_options.targets.browsers,
},
}, &import_records) catch |e| {
bun.handleErrorReturnTrace(e, @errorReturnTrace());
return .undefined;
}, &import_records)) {
.result => |result| result,
.err => |err| {
return err.toJSString(alloc, globalThis);
},
};

return bun.String.fromBytes(result.code).toJS(globalThis);
},
.err => |err| {
if (log.hasAny()) {
if (log.hasErrors()) {
return log.toJS(globalThis, bun.default_allocator, "parsing failed:");
}
return globalThis.throw("parsing failed: {}", .{err.kind});
},
}
}

fn parserOptionsFromJS(globalThis: *JSC.JSGlobalObject, allocator: Allocator, opts: *bun.css.ParserOptions, jsobj: JSValue) bun.JSError!void {
_ = allocator; // autofix
if (try jsobj.getTruthy(globalThis, "flags")) |val| {
if (val.isArray()) {
var iter = val.arrayIterator(globalThis);
while (iter.next()) |item| {
const bunstr = item.toBunString(globalThis);
defer bunstr.deref();
const str = bunstr.toUTF8(bun.default_allocator);
defer str.deinit();
if (std.mem.eql(u8, str.slice(), "DEEP_SELECTOR_COMBINATOR")) {
opts.flags.insert(bun.css.ParserFlags{
.deep_selector_combinator = true,
});
} else {
return globalThis.throw("invalid flag: {s}", .{str.slice()});
}
}
} else {
return globalThis.throw("flags must be an array", .{});
}
}

// if (try jsobj.getTruthy(globalThis, "css_modules")) |val| {
// opts.css_modules = bun.css.css_modules.Config{

// };
// if (val.isObject()) {
// if (try val.getTruthy(globalThis, "pure")) |pure_val| {
// opts.css_modules.pure = pure_val.toBoolean();
// }
// }
// }
}

fn targetsFromJS(globalThis: *JSC.JSGlobalObject, jsobj: JSValue) bun.JSError!bun.css.targets.Browsers {
var targets = bun.css.targets.Browsers{};

Expand Down
32 changes: 24 additions & 8 deletions src/css/css_parser.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2849,10 +2849,23 @@ pub fn StyleSheet(comptime AtRule: type) type {
return .{ .result = {} };
}

pub fn toCssWithWriter(this: *const @This(), allocator: Allocator, writer: anytype, options: css_printer.PrinterOptions, import_records: ?*const bun.BabyList(ImportRecord)) PrintErr!ToCssResultInternal {
pub fn toCssWithWriter(this: *const @This(), allocator: Allocator, writer: anytype, options: css_printer.PrinterOptions, import_records: ?*const bun.BabyList(ImportRecord)) PrintResult(ToCssResultInternal) {
const W = @TypeOf(writer);
const project_root = options.project_root;

var printer = Printer(@TypeOf(writer)).new(allocator, std.ArrayList(u8).init(allocator), writer, options, import_records);
const result = this.toCssWithWriterImpl(allocator, W, &printer, options, import_records) catch {
bun.assert(printer.error_kind != null);
return .{
.err = printer.error_kind.?,
};
};

return .{ .result = result };
}

pub fn toCssWithWriterImpl(this: *const @This(), allocator: Allocator, comptime W: type, printer: *Printer(W), options: css_printer.PrinterOptions, import_records: ?*const bun.BabyList(ImportRecord)) PrintErr!ToCssResultInternal {
_ = import_records; // autofix
const project_root = options.project_root;

// #[cfg(feature = "sourcemap")]
// {
Expand All @@ -2875,7 +2888,7 @@ pub fn StyleSheet(comptime AtRule: type) type {
var references = CssModuleReferences{};
printer.css_module = CssModule.new(allocator, config, &this.sources, project_root, &references);

try this.rules.toCss(W, &printer);
try this.rules.toCss(W, printer);
try printer.newline();

return ToCssResultInternal{
Expand All @@ -2889,7 +2902,7 @@ pub fn StyleSheet(comptime AtRule: type) type {
.references = references,
};
} else {
try this.rules.toCss(W, &printer);
try this.rules.toCss(W, printer);
try printer.newline();
return ToCssResultInternal{
.dependencies = printer.dependencies,
Expand All @@ -2900,18 +2913,21 @@ pub fn StyleSheet(comptime AtRule: type) type {
}
}

pub fn toCss(this: *const @This(), allocator: Allocator, options: css_printer.PrinterOptions, import_records: ?*const bun.BabyList(ImportRecord)) PrintErr!ToCssResult {
pub fn toCss(this: *const @This(), allocator: Allocator, options: css_printer.PrinterOptions, import_records: ?*const bun.BabyList(ImportRecord)) PrintResult(ToCssResult) {
// TODO: this is not necessary
// Make sure we always have capacity > 0: https://github.com/napi-rs/napi-rs/issues/1124.
var dest = ArrayList(u8).initCapacity(allocator, 1) catch unreachable;
const writer = dest.writer(allocator);
const result = try toCssWithWriter(this, allocator, writer, options, import_records);
return ToCssResult{
const result = switch (toCssWithWriter(this, allocator, writer, options, import_records)) {
.result => |v| v,
.err => |e| return .{ .err = e },
};
return .{ .result = ToCssResult{
.code = dest.items,
.dependencies = result.dependencies,
.exports = result.exports,
.references = result.references,
};
} };
}

pub fn parse(allocator: Allocator, code: []const u8, options: ParserOptions, import_records: ?*bun.BabyList(ImportRecord)) Maybe(This, Err(ParserError)) {
Expand Down
31 changes: 31 additions & 0 deletions src/css/error.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,26 @@ pub fn Err(comptime T: type) type {
/// The location where the error occurred.
loc: ?ErrorLocation,

pub fn fmt(
this: @This(),
comptime _: []const u8,
_: std.fmt.FormatOptions,
writer: anytype,
) !void {
if (@hasDecl(T, "fmt")) {
try writer.print("{}", .{this.kind});
return;
}
@compileError("fmt not implemented for " ++ @typeName(T));
}

pub fn toJSString(this: @This(), allocator: Allocator, globalThis: *bun.JSC.JSGlobalObject) bun.JSC.JSValue {
var error_string = ArrayList(u8){};
defer error_string.deinit(allocator);
error_string.writer(allocator).print("{}", .{this.kind}) catch unreachable;
return bun.String.fromBytes(error_string.items).toJS(globalThis);

@Jarred-Sumner Jarred-Sumner Jan 19, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

how about:

pub fn toString(this: @This()) !bun.String {
   return try bun.String.createFormat("{}", .{this});
}

pub fn toJS(this: @This(), globalObject: *JSC.JSGlobalObject) !JSC.JSValue {
   var str = try this.toString();
   return str.transferToJS();
}

}

pub fn fromParseError(err: ParseError(ParserError), filename: []const u8) Err(ParserError) {
if (T != ParserError) {
@compileError("Called .fromParseError() when T is not ParserError");
Expand Down Expand Up @@ -191,6 +211,17 @@ pub const PrinterErrorKind = union(enum) {
/// The CSS modules pattern must end with `[local]` for use in CSS grid.
invalid_css_modules_pattern_in_grid,
no_import_records,

pub fn format(this: @This(), comptime _: []const u8, _: std.fmt.FormatOptions, writer: anytype) !void {
return switch (this) {
.ambiguous_url_in_custom_property => |data| writer.print("Ambiguous relative URL '{s}' in custom property declaration", .{data.url}),
.fmt_error => writer.writeAll("Formatting error occurred"),
.invalid_composes_nesting => writer.writeAll("The 'composes' property cannot be used within nested rules"),
.invalid_composes_selector => writer.writeAll("The 'composes' property can only be used with a simple class selector"),
.invalid_css_modules_pattern_in_grid => writer.writeAll("CSS modules pattern must end with '[local]' when used in CSS grid"),
.no_import_records => writer.writeAll("No import records found"),
};
}
};

/// A parser error.
Expand Down
14 changes: 14 additions & 0 deletions src/css/media_query.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,16 @@ pub const MediaList = struct {
break :brk true;
};
}

/// Returns whether the media query list never matches.
pub fn neverMatches(this: *const MediaList) bool {
return this.media_queries.items.len > 0 and brk: {
for (this.media_queries.items) |*query| {
if (!query.neverMatches()) break :brk false;
}
break :brk true;
};
}
};

/// A binary `and` or `or` operator.
Expand Down Expand Up @@ -251,6 +261,10 @@ pub const MediaQuery = struct {

return toCssWithParensIfNeeded(condition, W, dest, needs_parens);
}

pub fn neverMatches(this: *const MediaQuery) bool {
return this.qualifier == .not and this.media_type == .all and this.condition == null;
}
};

/// Flags for `parse_query_condition`.
Expand Down
Loading