Skip to content

Commit

Permalink
fix #469: treat package paths in css as relative
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Oct 19, 2020
1 parent 99a787b commit 588fa61
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 13 deletions.
12 changes: 10 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
# Changelog

## Unreleased

* Treat paths in CSS without a `./` or `../` prefix as relative ([#469](https://github.com/evanw/esbuild/issues/469))

JavaScript paths starting with `./` or `../` are considered relative paths, while other JavaScript paths are considered package paths and are looked up in that package's `node_modules` directory. Currently `url()` paths in CSS files use that same logic, so `url(images/image.png)` checks for a file named `image.png` in the `image` package.

This release changes this behavior. Now `url(images/image.png)` first checks for `./images/image.png`, then checks for a file named `image.png` in the `image` package. This behavior should match the behavior of Webpack's standard `css-loader` package.

## 0.7.17

* Add `--public-path=` for the `file` loader ([#459](https://github.com/evanw/esbuild/issues/459))
Expand Down Expand Up @@ -1140,7 +1148,7 @@

* Fixed ordering between `node_modules` and a force-overridden `tsconfig.json` ([#278](https://github.com/evanw/esbuild/issues/278))

When the `tsconfig.json` settings have been force-overridden using the new `--tsconfig` flag, the path resolution behavior behaved subtly differently than if esbuild naturally discovers the `tsconfig.json` file without the flag. The difference caused package paths present in a `node_modules` folder to incorrectly take precedence over custom path aliases configured in `tsconfig.json`. The ordering has been corrected such that custom path aliases always take place over `node_modules`.
When the `tsconfig.json` settings have been force-overridden using the new `--tsconfig` flag, the path resolution behavior behaved subtly differently than if esbuild naturally discovers the `tsconfig.json` file without the flag. The difference caused package paths present in a `node_modules` directory to incorrectly take precedence over custom path aliases configured in `tsconfig.json`. The ordering has been corrected such that custom path aliases always take place over `node_modules`.

* Add the `--out-extension` flag for custom output extensions ([#281](https://github.com/evanw/esbuild/issues/281))

Expand Down Expand Up @@ -1981,7 +1989,7 @@ Note that you can also just use `--strict` to enable strictness for all transfor
* Add the `file` loader ([#14](https://github.com/evanw/esbuild/issues/14) and [#135](https://github.com/evanw/esbuild/pull/135))
The `file` loader copies the input file to the output folder and exports the path of the file as a string to any modules that import the file. For example, `--loader:.png=file` enables this loader for all imported `.png` files. This was contributed by [@viankakrisna](https://github.com/viankakrisna).
The `file` loader copies the input file to the output directory and exports the path of the file as a string to any modules that import the file. For example, `--loader:.png=file` enables this loader for all imported `.png` files. This was contributed by [@viankakrisna](https://github.com/viankakrisna).
* Add the `--resolve-extensions` flag ([#142](https://github.com/evanw/esbuild/pull/142))
Expand Down
6 changes: 3 additions & 3 deletions internal/ast/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ const (
ImportRequireResolve

// A CSS "@import" rule
AtImport
ImportAt

// A CSS "url(...)" token
URLToken
ImportURL
)

func (kind ImportKind) IsFromCSS() bool {
return kind == AtImport || kind == URLToken
return kind == ImportAt || kind == ImportURL
}

type ImportRecord struct {
Expand Down
4 changes: 2 additions & 2 deletions internal/bundler/bundler.go
Original file line number Diff line number Diff line change
Expand Up @@ -801,14 +801,14 @@ func ScanBundle(log logger.Log, fs fs.FS, res resolver.Resolver, entryPaths []st

// Importing a JavaScript file from a CSS file is not allowed.
switch record.Kind {
case ast.AtImport:
case ast.ImportAt:
otherFile := &results[*record.SourceIndex].file
if _, ok := otherFile.repr.(*reprJS); ok {
log.AddRangeError(&result.file.source, record.Range,
fmt.Sprintf("Cannot import %q into a CSS file", otherFile.source.PrettyPath))
}

case ast.URLToken:
case ast.ImportURL:
otherFile := &results[*record.SourceIndex].file
switch otherRepr := otherFile.repr.(type) {
case *reprCSS:
Expand Down
25 changes: 25 additions & 0 deletions internal/bundler/bundler_css_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -452,3 +452,28 @@ func TestIgnoreURLsInAtRulePrelude(t *testing.T) {
},
})
}

func TestPackageURLsInCSS(t *testing.T) {
css_suite.expectBundled(t, bundled{
files: map[string]string{
"/entry.css": `
a { background: url(a/1.png); }
b { background: url(b/2.png); }
c { background: url(c/3.png); }
`,
"/a/1.png": `a-1`,
"/node_modules/b/2.png": `b-2-node_modules`,
"/c/3.png": `c-3`,
"/node_modules/c/3.png": `c-3-node_modules`,
},
entryPaths: []string{"/entry.css"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputDir: "/out",
ExtensionToLoader: map[string]config.Loader{
".css": config.LoaderCSS,
".png": config.LoaderBase64,
},
},
})
}
14 changes: 14 additions & 0 deletions internal/bundler/snapshots/snapshots_css.txt
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,20 @@ console.log("b");
color: blue;
}

================================================================================
TestPackageURLsInCSS
---------- /out/entry.css ----------
/* /entry.css */
a {
background: url(data:image/png;base64,YS0x);
}
b {
background: url(data:image/png;base64,Yi0yLW5vZGVfbW9kdWxlcw==);
}
c {
background: url(data:image/png;base64,Yy0z);
}

================================================================================
TestTextImportURLInCSSText
---------- /out/entry.css ----------
Expand Down
6 changes: 3 additions & 3 deletions internal/css_parser/css_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ func (p *parser) parseAtRule(context atRuleContext) css_ast.R {
p.expect(css_lexer.TSemicolon)
importRecordIndex := uint32(len(p.importRecords))
p.importRecords = append(p.importRecords, ast.ImportRecord{
Kind: ast.AtImport,
Kind: ast.ImportAt,
Path: logger.Path{Text: path},
Range: r,
})
Expand Down Expand Up @@ -544,7 +544,7 @@ loop:
token.Text = ""
token.ImportRecordIndex = uint32(len(p.importRecords))
p.importRecords = append(p.importRecords, ast.ImportRecord{
Kind: ast.URLToken,
Kind: ast.ImportURL,
Path: logger.Path{Text: path},
Range: r,
IsUnused: !allowImports,
Expand All @@ -566,7 +566,7 @@ loop:
token.Children = nil
token.ImportRecordIndex = uint32(len(p.importRecords))
p.importRecords = append(p.importRecords, ast.ImportRecord{
Kind: ast.URLToken,
Kind: ast.ImportURL,
Path: logger.Path{Text: arg.Text},
Range: original[0].Range,
IsUnused: !allowImports,
Expand Down
15 changes: 12 additions & 3 deletions internal/resolver/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,13 @@ func (r *resolver) resolveWithoutSymlinks(sourceDir string, importPath string, k
return &ResolveResult{PathPair: PathPair{Primary: logger.Path{Text: importPath, Namespace: "file"}}}
}

if !IsPackagePath(importPath) {
// Check both relative and package paths for CSS URL tokens, with relative
// paths taking precedence over package paths to match Webpack behavior.
isPackagePath := IsPackagePath(importPath)
checkRelative := !isPackagePath || kind == ast.ImportURL
checkPackage := isPackagePath

if checkRelative {
absPath := r.fs.Join(sourceDir, importPath)

// Check for external packages first
Expand All @@ -269,11 +275,14 @@ func (r *resolver) resolveWithoutSymlinks(sourceDir string, importPath string, k
}

if absolute, ok := r.loadAsFileOrDirectory(absPath, kind); ok {
checkPackage = false
result = absolute
} else {
} else if !checkPackage {
return nil
}
} else {
}

if checkPackage {
// Check for external packages first
if r.options.ExternalModules.NodeModules != nil {
query := importPath
Expand Down

0 comments on commit 588fa61

Please sign in to comment.