Skip to content

Commit

Permalink
fix #3286: don't wrap strings with unique keys
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Aug 5, 2023
1 parent 691e81d commit 441d6df
Show file tree
Hide file tree
Showing 10 changed files with 140 additions and 49 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

* Fix asset references with the `--line-limit` flag ([#3286](https://github.com/evanw/esbuild/issues/3286))

The recently-released `--line-limit` flag tells esbuild to terminate long lines after they pass this length limit. This includes automatically wrapping long strings across multiple lines using escaped newline syntax. However, using this could cause esbuild to generate incorrect code for references from generated output files to assets in the bundle (i.e. files loaded with the `file` or `copy` loaders). This is because esbuild implements asset references internally using find-and-replace with a randomly-generated string, but the find operation fails if the string is split by an escaped newline due to line wrapping. This release fixes the problem by not wrapping these strings. This issue affected asset references in both JS and CSS files.

* Support local names in CSS for `@keyframe`, `@counter-style`, and `@container` ([#20](https://github.com/evanw/esbuild/issues/20))

This release extends support for local names in CSS files loaded with the `local-css` loader to cover the `@keyframe`, `@counter-style`, and `@container` rules (and also `animation`, `list-style`, and `container` declarations). Here's an example:
Expand Down
4 changes: 4 additions & 0 deletions internal/ast/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ const (

// CSS "@import" of an empty file should be removed
WasLoadedWithEmptyLoader

// Unique keys are randomly-generated strings that are used to replace paths
// in the source code after it's printed. These must not ever be split apart.
ContainsUniqueKey
)

func (flags ImportRecordFlags) Has(flag ImportRecordFlags) bool {
Expand Down
5 changes: 4 additions & 1 deletion internal/bundler/bundler.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,10 @@ func parseFile(args parseArgs) {
case config.LoaderFile:
uniqueKey := fmt.Sprintf("%sA%08d", args.uniqueKeyPrefix, args.sourceIndex)
uniqueKeyPath := uniqueKey + source.KeyPath.IgnoredSuffix
expr := js_ast.Expr{Data: &js_ast.EString{Value: helpers.StringToUTF16(uniqueKeyPath)}}
expr := js_ast.Expr{Data: &js_ast.EString{
Value: helpers.StringToUTF16(uniqueKeyPath),
ContainsUniqueKey: true,
}}
ast := js_parser.LazyExportAST(args.log, source, js_parser.OptionsFromConfig(&args.options), expr, "")
ast.URLForCSS = uniqueKeyPath
if pluginName != "" {
Expand Down
20 changes: 20 additions & 0 deletions internal/bundler_tests/bundler_default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8321,6 +8321,9 @@ func TestLineLimitNotMinified(t *testing.T) {
default_suite.expectBundled(t, bundled{
files: map[string]string{
"/script.jsx": `
import fileURL from './x.file'
import copyURL from './x.copy'
import dataURL from './x.data'
export const SignUpForm = (props) => {
return <p class="signup">
<label>Username: <input class="username" type="text"/></label>
Expand All @@ -8329,6 +8332,9 @@ func TestLineLimitNotMinified(t *testing.T) {
{props.buttonText}
</div>
<small>By signing up, you are agreeing to our <a href="/tos/">terms of service</a>.</small>
<img src={fileURL} />
<img src={copyURL} />
<img src={dataURL} />
</p>
}
`,
Expand All @@ -8343,16 +8349,30 @@ func TestLineLimitNotMinified(t *testing.T) {
`CIgZmlsbD0iI0ZGQ0YwMCIvPgogIDxwYXRoIGQ9Ik00Ny41IDUyLjVMOTUgMTAwbC00Ny41IDQ3LjVtNjAtOTVMM` +
`TU1IDEwMGwtNDcuNSA0Ny41IiBmaWxsPSJub25lIiBzdHJva2U9IiMxOTE5MTkiIHN0cm9rZS13aWR0aD0iMjQiL` +
`z4KPC9zdmc+Cg==);
cursor: url(x.file);
cursor: url(x.copy);
cursor: url(x.data);
}
`,
"/x.file": `...file...`,
"/x.copy": `...copy...`,
"/x.data": `...lots of long data...lots of long data...`,
},
entryPaths: []string{
"/script.jsx",
"/style.css",
},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputDir: "/out",
LineLimit: 32,
ExtensionToLoader: map[string]config.Loader{
".jsx": config.LoaderJSX,
".css": config.LoaderCSS,
".file": config.LoaderFile,
".copy": config.LoaderCopy,
".data": config.LoaderDataURL,
},
},
})
}
Expand Down
34 changes: 32 additions & 2 deletions internal/bundler_tests/snapshots/snapshots_default.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2979,8 +2979,24 @@ BzdHJva2U9IiMxOTE5MTkiIHN0cm9rZS\

================================================================================
TestLineLimitNotMinified
---------- /out/x-TZ25B4WH.file ----------
...file...
---------- /out/x-UF3O47Y3.copy ----------
...copy...
---------- /out/script.js ----------
export const SignUpForm = (props) => {
// x.file
var x_default = "./x-TZ25B4WH.file";

// script.jsx
import copyURL from "./x-UF3O47Y3.copy";

// x.data
var x_default2 = "data:text/plai\
n;charset=utf-8,...lots of long \
data...lots of long data...";

// script.jsx
var SignUpForm = (props) => {
return /* @__PURE__ */ React.createElement(
"p", { class: "signup" }, /* @__PURE__ */ React.
createElement("label", null, "\
Expand All @@ -2998,10 +3014,19 @@ led" }, props.buttonText), /* @__PURE__ */ React.
By signing up, you are agreeing \
to our ", /* @__PURE__ */ React.
createElement("a", { href: "/t\
os/" }, "terms of service"), "."));
os/" }, "terms of service"), "."),
/* @__PURE__ */ React.createElement(
"img", { src: x_default }), /* @__PURE__ */ React.
createElement("img", { src: copyURL }),
/* @__PURE__ */ React.createElement(
"img", { src: x_default2 }));
};
export {
SignUpForm
};

---------- /out/style.css ----------
/* style.css */
body.light-mode.new-user-segment:not(.logged-in)
.signup,
body.light-mode.new-user-segment:not(.logged-in)
Expand All @@ -3027,6 +3052,11 @@ tNjAtOTVMMTU1IDEwMGwtNDcuNSA0Ny4\
1IiBmaWxsPSJub25lIiBzdHJva2U9IiM\
xOTE5MTkiIHN0cm9rZS13aWR0aD0iMjQ\
iLz4KPC9zdmc+Cg==");
cursor: url(./x-TZ25B4WH.file);
cursor: url(./x-UF3O47Y3.copy);
cursor: url("data:text/plain;c\
harset=utf-8,...lots of long dat\
a...lots of long data...");
}

================================================================================
Expand Down
37 changes: 26 additions & 11 deletions internal/css_printer/css_printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func (p *printer) printRule(rule css_ast.Rule, indent int32, omitTrailingSemicol
p.print("@charset ")

// It's not valid to print the string with single quotes
p.printQuotedWithQuote(r.Encoding, '"')
p.printQuotedWithQuote(r.Encoding, '"', 0)
p.print(";")

case *css_ast.RAtImport:
Expand All @@ -163,7 +163,12 @@ func (p *printer) printRule(rule css_ast.Rule, indent int32, omitTrailingSemicol
} else {
p.print("@import ")
}
p.printQuoted(p.importRecords[r.ImportRecordIndex].Path.Text)
record := p.importRecords[r.ImportRecordIndex]
var flags printQuotedFlags
if record.Flags.Has(ast.ContainsUniqueKey) {
flags |= printQuotedNoWrap
}
p.printQuoted(record.Path.Text, flags)
p.recordImportPathForMetafile(r.ImportRecordIndex)
p.printTokens(r.ImportConditions, printTokensOpts{})
p.print(";")
Expand Down Expand Up @@ -485,7 +490,7 @@ func (p *printer) printCompoundSelector(sel css_ast.CompoundSelector, isFirst bo
if printAsIdent {
p.printIdent(s.MatcherValue, identNormal, canDiscardWhitespaceAfter)
} else {
p.printQuoted(s.MatcherValue)
p.printQuoted(s.MatcherValue, 0)
}
}
if s.MatcherModifier != 0 {
Expand Down Expand Up @@ -633,8 +638,14 @@ func bestQuoteCharForString(text string, forURL bool) byte {
return '"'
}

func (p *printer) printQuoted(text string) {
p.printQuotedWithQuote(text, bestQuoteCharForString(text, false))
type printQuotedFlags uint8

const (
printQuotedNoWrap printQuotedFlags = 1 << iota
)

func (p *printer) printQuoted(text string, flags printQuotedFlags) {
p.printQuotedWithQuote(text, bestQuoteCharForString(text, false), flags)
}

type escapeKind uint8
Expand Down Expand Up @@ -685,7 +696,7 @@ func (p *printer) printWithEscape(c rune, escape escapeKind, remainingText strin
}

// Note: This function is hot in profiles
func (p *printer) printQuotedWithQuote(text string, quote byte) {
func (p *printer) printQuotedWithQuote(text string, quote byte, flags printQuotedFlags) {
if quote != quoteForURL {
p.css = append(p.css, quote)
}
Expand All @@ -697,7 +708,7 @@ func (p *printer) printQuotedWithQuote(text string, quote byte) {
// Only compute the line length if necessary
var startLineLength int
wrapLongLines := false
if p.options.LineLimit > 0 && quote != quoteForURL {
if p.options.LineLimit > 0 && quote != quoteForURL && (flags&printQuotedNoWrap) == 0 {
startLineLength = p.currentLineLength()
if startLineLength > p.options.LineLimit {
startLineLength = p.options.LineLimit
Expand Down Expand Up @@ -983,16 +994,20 @@ func (p *printer) printTokens(tokens []css_ast.Token, opts printTokensOpts) bool
p.printIdent(t.Text, identHash, whitespace)

case css_lexer.TString:
p.printQuoted(t.Text)
p.printQuoted(t.Text, 0)

case css_lexer.TURL:
text := p.importRecords[t.PayloadIndex].Path.Text
record := p.importRecords[t.PayloadIndex]
text := record.Path.Text
tryToAvoidQuote := true
if p.options.LineLimit > 0 && p.currentLineLength()+len(text) >= p.options.LineLimit {
var flags printQuotedFlags
if record.Flags.Has(ast.ContainsUniqueKey) {
flags |= printQuotedNoWrap
} else if p.options.LineLimit > 0 && p.currentLineLength()+len(text) >= p.options.LineLimit {
tryToAvoidQuote = false
}
p.print("url(")
p.printQuotedWithQuote(text, bestQuoteCharForString(text, tryToAvoidQuote))
p.printQuotedWithQuote(text, bestQuoteCharForString(text, tryToAvoidQuote), flags)
p.print(")")
p.recordImportPathForMetafile(t.PayloadIndex)

Expand Down
2 changes: 1 addition & 1 deletion internal/css_printer/css_printer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func expectPrintedString(t *testing.T, stringValue string, expected string) {
t.Run(stringValue, func(t *testing.T) {
t.Helper()
p := printer{}
p.printQuoted(stringValue)
p.printQuoted(stringValue, 0)
test.AssertEqualWithDiff(t, string(p.css), expected)
})
}
Expand Down
1 change: 1 addition & 0 deletions internal/js_ast/js_ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -779,6 +779,7 @@ type EString struct {
LegacyOctalLoc logger.Loc
PreferTemplate bool
HasPropertyKeyComment bool // If true, a preceding comment contains "@__KEY__"
ContainsUniqueKey bool // If true, this string must not be wrapped
}

type TemplatePart struct {
Expand Down
Loading

0 comments on commit 441d6df

Please sign in to comment.