Skip to content

Commit

Permalink
initial support for conditional css imports (#953)
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Mar 11, 2021
1 parent cf2f34c commit a33449b
Show file tree
Hide file tree
Showing 13 changed files with 116 additions and 18 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 4 additions & 1 deletion internal/ast/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ const (
// A CSS "@import" rule
ImportAt

// A CSS "@import" rule with import conditions
ImportAtConditional

// A CSS "url(...)" token
ImportURL
)
Expand All @@ -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"
Expand Down
5 changes: 4 additions & 1 deletion internal/bundler/bundler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
42 changes: 42 additions & 0 deletions internal/bundler/bundler_css_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
`,
})
}
27 changes: 19 additions & 8 deletions internal/bundler/linker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
}
}

Expand Down
12 changes: 12 additions & 0 deletions internal/bundler/snapshots/snapshots_css.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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 ----------
Expand Down
1 change: 1 addition & 0 deletions internal/css_ast/css_ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ type RAtCharset struct {

type RAtImport struct {
ImportRecordIndex uint32
ImportConditions []Token
}

type RAtKeyframes struct {
Expand Down
25 changes: 22 additions & 3 deletions internal/css_parser/css_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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":
Expand Down
6 changes: 3 additions & 3 deletions internal/css_parser/css_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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;", "<stdin>: warning: Expected URL token but found \";\"\n")
expectParseError(t, "@import ;", "<stdin>: warning: Expected URL token but found \";\"\n")
Expand All @@ -714,9 +716,7 @@ func TestAtImport(t *testing.T) {
<stdin>: warning: Expected ";" but found end of file
`)

expectParseError(t, "@import \"foo.css\" {}", `<stdin>: warning: Expected ";"
<stdin>: warning: Unexpected "{"
`)
expectParseError(t, "@import \"foo.css\" {}", "<stdin>: warning: Expected \";\" but found end of file\n")
}

func TestAtKeyframes(t *testing.T) {
Expand Down
1 change: 1 addition & 0 deletions internal/css_printer/css_printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions internal/css_printer/css_printer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion internal/resolver/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/api/api_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit a33449b

Please sign in to comment.