From a33449bba69b5e46eba8c0dc89604b42fd46073d Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Thu, 11 Mar 2021 10:11:21 -0800 Subject: [PATCH] initial support for conditional css imports (#953) --- CHANGELOG.md | 4 ++ internal/ast/ast.go | 5 ++- internal/bundler/bundler.go | 5 ++- internal/bundler/bundler_css_test.go | 42 ++++++++++++++++++++ internal/bundler/linker.go | 27 +++++++++---- internal/bundler/snapshots/snapshots_css.txt | 12 ++++++ internal/css_ast/css_ast.go | 1 + internal/css_parser/css_parser.go | 25 ++++++++++-- internal/css_parser/css_parser_test.go | 6 +-- internal/css_printer/css_printer.go | 1 + internal/css_printer/css_printer_test.go | 2 + internal/resolver/resolver.go | 2 +- pkg/api/api_impl.go | 2 +- 13 files changed, 116 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 28050b5a988..7dcb2af4829 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ Previously bundling with esbuild when a parent directory is inaccessible did not work because esbuild would try to read the directory to search for a `node_modules` folder and would then fail the build when that failed. In practice this caused issues in certain Linux environments where a directory close to the root directory was inaccessible (e.g. on Android). With this release, esbuild will treat inaccessible directories as empty to allow for the `node_modules` search to continue past the inaccessible directory and into its parent directory. This means it should now be possible to bundle with esbuild in these situations. +* Support conditional `@import` syntax when not bundling ([#953](https://github.com/evanw/esbuild/issues/953)) + + Previously conditional CSS imports such as `@import "print.css" print;` was not supported at all and was considered a syntax error. With this release, it is now supported in all cases except when bundling an internal import. Support for bundling internal CSS imports is planned but will happen in a later release. + ## 0.9.0 **This release contains backwards-incompatible changes.** Since esbuild is before version 1.0.0, these changes have been released as a new minor version to reflect this (as [recommended by npm](https://docs.npmjs.com/cli/v6/using-npm/semver/)). You should either be pinning the exact version of `esbuild` in your `package.json` file or be using a version range syntax that only accepts patch upgrades such as `^0.8.0`. See the documentation about [semver](https://docs.npmjs.com/cli/v6/using-npm/semver/) for more information. diff --git a/internal/ast/ast.go b/internal/ast/ast.go index 23721264550..879e80f67bf 100644 --- a/internal/ast/ast.go +++ b/internal/ast/ast.go @@ -27,6 +27,9 @@ const ( // A CSS "@import" rule ImportAt + // A CSS "@import" rule with import conditions + ImportAtConditional + // A CSS "url(...)" token ImportURL ) @@ -41,7 +44,7 @@ func (kind ImportKind) StringForMetafile() string { return "dynamic-import" case ImportRequireResolve: return "require-resolve" - case ImportAt: + case ImportAt, ImportAtConditional: return "import-rule" case ImportURL: return "url-token" diff --git a/internal/bundler/bundler.go b/internal/bundler/bundler.go index 1a5adb590fd..213cf0ed4e5 100644 --- a/internal/bundler/bundler.go +++ b/internal/bundler/bundler.go @@ -1365,12 +1365,15 @@ func (s *scanner) processScannedFiles() []file { } switch record.Kind { - case ast.ImportAt: + case ast.ImportAt, ast.ImportAtConditional: // Using a JavaScript file with CSS "@import" is not allowed otherFile := &s.results[record.SourceIndex.GetIndex()].file if _, ok := otherFile.repr.(*reprJS); ok { s.log.AddRangeError(&result.file.source, record.Range, fmt.Sprintf("Cannot import %q into a CSS file", otherFile.source.PrettyPath)) + } else if record.Kind == ast.ImportAtConditional { + s.log.AddRangeError(&result.file.source, record.Range, + "Bundling with conditional \"@import\" rules is not currently supported") } case ast.ImportURL: diff --git a/internal/bundler/bundler_css_test.go b/internal/bundler/bundler_css_test.go index 12ec8328418..ebcf0e5c322 100644 --- a/internal/bundler/bundler_css_test.go +++ b/internal/bundler/bundler_css_test.go @@ -521,3 +521,45 @@ func TestCSSAtImportExtensionOrderCollisionUnsupported(t *testing.T) { `, }) } + +func TestCSSAtImportConditionsNoBundle(t *testing.T) { + css_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.css": `@import "./print.css" print;`, + }, + entryPaths: []string{"/entry.css"}, + options: config.Options{ + Mode: config.ModePassThrough, + AbsOutputFile: "/out.css", + }, + }) +} + +func TestCSSAtImportConditionsBundleExternal(t *testing.T) { + css_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.css": `@import "https://example.com/print.css" print;`, + }, + entryPaths: []string{"/entry.css"}, + options: config.Options{ + Mode: config.ModeBundle, + AbsOutputFile: "/out.css", + }, + }) +} + +func TestCSSAtImportConditionsBundle(t *testing.T) { + css_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.css": `@import "./print.css" print;`, + "/print.css": `body { color: red }`, + }, + entryPaths: []string{"/entry.css"}, + options: config.Options{ + Mode: config.ModeBundle, + AbsOutputFile: "/out.css", + }, + expectedScanLog: `entry.css: error: Bundling with conditional "@import" rules is not currently supported +`, + }) +} diff --git a/internal/bundler/linker.go b/internal/bundler/linker.go index cab7e6c0c01..be7fe83886b 100644 --- a/internal/bundler/linker.go +++ b/internal/bundler/linker.go @@ -4130,10 +4130,15 @@ func (c *linkerContext) generateGlobalNamePrefix() string { } type compileResultCSS struct { - printedCSS string - sourceIndex uint32 - hasCharset bool - externalImportRecords []ast.ImportRecord + printedCSS string + sourceIndex uint32 + hasCharset bool + externalImports []externalImportCSS +} + +type externalImportCSS struct { + record ast.ImportRecord + conditions []css_ast.Token } func (repr *chunkReprCSS) generate(c *linkerContext, chunk *chunkInfo) func(generateContinue) []OutputFile { @@ -4164,7 +4169,10 @@ func (repr *chunkReprCSS) generate(c *linkerContext, chunk *chunkInfo) func(gene continue case *css_ast.RAtImport: if record := ast.ImportRecords[r.ImportRecordIndex]; !record.SourceIndex.IsValid() { - compileResult.externalImportRecords = append(compileResult.externalImportRecords, record) + compileResult.externalImports = append(compileResult.externalImports, externalImportCSS{ + record: record, + conditions: r.ImportConditions, + }) } continue } @@ -4208,9 +4216,12 @@ func (repr *chunkReprCSS) generate(c *linkerContext, chunk *chunkInfo) func(gene // Insert all external "@import" rules at the front. In CSS, all "@import" // rules must come first or the browser will just ignore them. for _, compileResult := range compileResults { - for _, record := range compileResult.externalImportRecords { - ast.Rules = append(ast.Rules, &css_ast.RAtImport{ImportRecordIndex: uint32(len(ast.ImportRecords))}) - ast.ImportRecords = append(ast.ImportRecords, record) + for _, external := range compileResult.externalImports { + ast.Rules = append(ast.Rules, &css_ast.RAtImport{ + ImportRecordIndex: uint32(len(ast.ImportRecords)), + ImportConditions: external.conditions, + }) + ast.ImportRecords = append(ast.ImportRecords, external.record) } } diff --git a/internal/bundler/snapshots/snapshots_css.txt b/internal/bundler/snapshots/snapshots_css.txt index e903f07af19..90760af8d0e 100644 --- a/internal/bundler/snapshots/snapshots_css.txt +++ b/internal/bundler/snapshots/snapshots_css.txt @@ -36,6 +36,18 @@ TestCSSAtImport color: red; } +================================================================================ +TestCSSAtImportConditionsBundleExternal +---------- /out.css ---------- +@import "https://example.com/print.css" print; + +/* entry.css */ + +================================================================================ +TestCSSAtImportConditionsNoBundle +---------- /out.css ---------- +@import "./print.css" print; + ================================================================================ TestCSSAtImportExtensionOrderCollision ---------- /out.css ---------- diff --git a/internal/css_ast/css_ast.go b/internal/css_ast/css_ast.go index 6acb263cb2d..60bc4a817b4 100644 --- a/internal/css_ast/css_ast.go +++ b/internal/css_ast/css_ast.go @@ -98,6 +98,7 @@ type RAtCharset struct { type RAtImport struct { ImportRecordIndex uint32 + ImportConditions []Token } type RAtKeyframes struct { diff --git a/internal/css_parser/css_parser.go b/internal/css_parser/css_parser.go index cfb02787e65..5d1370657ca 100644 --- a/internal/css_parser/css_parser.go +++ b/internal/css_parser/css_parser.go @@ -336,15 +336,34 @@ func (p *parser) parseAtRule(context atRuleContext) css_ast.R { kind = atRuleEmpty p.eat(css_lexer.TWhitespace) if path, r, ok := p.expectURLOrString(); ok { - p.eat(css_lexer.TWhitespace) + importConditionsStart := p.index + for p.current().Kind != css_lexer.TSemicolon && p.current().Kind != css_lexer.TEndOfFile { + p.parseComponentValue() + } + importConditions := p.convertTokens(p.tokens[importConditionsStart:p.index]) + kind := ast.ImportAt + + // Insert or remove whitespace before the first token + if len(importConditions) > 0 { + kind = ast.ImportAtConditional + if p.options.RemoveWhitespace { + importConditions[0].Whitespace &= ^css_ast.WhitespaceBefore + } else { + importConditions[0].Whitespace |= css_ast.WhitespaceBefore + } + } + p.expect(css_lexer.TSemicolon) importRecordIndex := uint32(len(p.importRecords)) p.importRecords = append(p.importRecords, ast.ImportRecord{ - Kind: ast.ImportAt, + Kind: kind, Path: logger.Path{Text: path}, Range: r, }) - return &css_ast.RAtImport{ImportRecordIndex: importRecordIndex} + return &css_ast.RAtImport{ + ImportRecordIndex: importRecordIndex, + ImportConditions: importConditions, + } } case "keyframes", "-webkit-keyframes", "-moz-keyframes", "-ms-keyframes", "-o-keyframes": diff --git a/internal/css_parser/css_parser_test.go b/internal/css_parser/css_parser_test.go index ac19ddfe345..18191abcbb4 100644 --- a/internal/css_parser/css_parser_test.go +++ b/internal/css_parser/css_parser_test.go @@ -703,6 +703,8 @@ func TestAtImport(t *testing.T) { expectPrinted(t, "@import url(foo.css) ;", "@import \"foo.css\";\n") expectPrinted(t, "@import url(\"foo.css\");", "@import \"foo.css\";\n") expectPrinted(t, "@import url(\"foo.css\") ;", "@import \"foo.css\";\n") + expectPrinted(t, "@import url(\"foo.css\") print;", "@import \"foo.css\" print;\n") + expectPrinted(t, "@import url(\"foo.css\") screen and (orientation:landscape);", "@import \"foo.css\" screen and (orientation:landscape);\n") expectParseError(t, "@import;", ": warning: Expected URL token but found \";\"\n") expectParseError(t, "@import ;", ": warning: Expected URL token but found \";\"\n") @@ -714,9 +716,7 @@ func TestAtImport(t *testing.T) { : warning: Expected ";" but found end of file `) - expectParseError(t, "@import \"foo.css\" {}", `: warning: Expected ";" -: warning: Unexpected "{" -`) + expectParseError(t, "@import \"foo.css\" {}", ": warning: Expected \";\" but found end of file\n") } func TestAtKeyframes(t *testing.T) { diff --git a/internal/css_printer/css_printer.go b/internal/css_printer/css_printer.go index 6f8d4bfd572..8ac2ecd316a 100644 --- a/internal/css_printer/css_printer.go +++ b/internal/css_printer/css_printer.go @@ -56,6 +56,7 @@ func (p *printer) printRule(rule css_ast.R, indent int32, omitTrailingSemicolon p.print("@import ") } p.printQuoted(p.importRecords[r.ImportRecordIndex].Path.Text) + p.printTokens(r.ImportConditions, printTokensOpts{}) p.print(";") case *css_ast.RAtKeyframes: diff --git a/internal/css_printer/css_printer_test.go b/internal/css_printer/css_printer_test.go index 8a6942a0478..e0cdabde30b 100644 --- a/internal/css_printer/css_printer_test.go +++ b/internal/css_printer/css_printer_test.go @@ -306,11 +306,13 @@ func TestAtImport(t *testing.T) { expectPrinted(t, "@import \"foo.css\";", "@import \"foo.css\";\n") expectPrinted(t, "@import url(foo.css);", "@import \"foo.css\";\n") expectPrinted(t, "@import url(\"foo.css\");", "@import \"foo.css\";\n") + expectPrinted(t, "@import url(\"foo.css\") print;", "@import \"foo.css\" print;\n") expectPrintedMinify(t, "@import\"foo.css\";", "@import\"foo.css\";") expectPrintedMinify(t, "@import \"foo.css\";", "@import\"foo.css\";") expectPrintedMinify(t, "@import url(foo.css);", "@import\"foo.css\";") expectPrintedMinify(t, "@import url(\"foo.css\");", "@import\"foo.css\";") + expectPrintedMinify(t, "@import url(\"foo.css\") print;", "@import\"foo.css\"print;") } func TestAtKeyframes(t *testing.T) { diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index be3aa3608c5..dfb25c55d74 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -981,7 +981,7 @@ func getBool(json js_ast.Expr) (bool, bool) { func (r *resolver) loadAsFileOrDirectory(path string, kind ast.ImportKind) (PathPair, bool, *fs.DifferentCase) { // Use a special import order for CSS "@import" imports extensionOrder := r.options.ExtensionOrder - if kind == ast.ImportAt { + if kind == ast.ImportAt || kind == ast.ImportAtConditional { extensionOrder = r.atImportExtensionOrder } diff --git a/pkg/api/api_impl.go b/pkg/api/api_impl.go index 9b0ea30177e..5214fca9223 100644 --- a/pkg/api/api_impl.go +++ b/pkg/api/api_impl.go @@ -1277,7 +1277,7 @@ func (impl *pluginImpl) OnResolve(options OnResolveOptions, callback func(OnReso kind = ResolveJSDynamicImport case ast.ImportRequireResolve: kind = ResolveJSRequireResolve - case ast.ImportAt: + case ast.ImportAt, ast.ImportAtConditional: kind = ResolveCSSImportRule case ast.ImportURL: kind = ResolveCSSURLToken