From 98bd7c37018c08f26a08c4a00c25315b971ed8b7 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Wed, 4 Jan 2023 14:45:10 -0500 Subject: [PATCH] preserve comments in import assertions too --- internal/ast/ast.go | 11 +- internal/bundler/bundler.go | 2 +- .../bundler_tests/bundler_default_test.go | 35 ++++++ .../snapshots/snapshots_default.txt | 58 +++++++++ internal/js_parser/js_parser.go | 35 ++++-- internal/js_printer/js_printer.go | 116 +++++++++++++++--- 6 files changed, 230 insertions(+), 27 deletions(-) diff --git a/internal/ast/ast.go b/internal/ast/ast.go index 3f037d51a49..9332a4f04cd 100644 --- a/internal/ast/ast.go +++ b/internal/ast/ast.go @@ -129,7 +129,7 @@ func (flags ImportRecordFlags) Has(flag ImportRecordFlags) bool { } type ImportRecord struct { - Assertions *[]AssertEntry + Assertions *ImportAssertions Path logger.Path Range logger.Range @@ -149,6 +149,15 @@ type ImportRecord struct { Kind ImportKind } +type ImportAssertions struct { + Entries []AssertEntry + AssertLoc logger.Loc + InnerOpenBraceLoc logger.Loc + InnerCloseBraceLoc logger.Loc + OuterOpenBraceLoc logger.Loc + OuterCloseBraceLoc logger.Loc +} + type AssertEntry struct { Key []uint16 // An identifier or a string Value []uint16 // Always a string diff --git a/internal/bundler/bundler.go b/internal/bundler/bundler.go index 6146a6d789d..133aa312b46 100644 --- a/internal/bundler/bundler.go +++ b/internal/bundler/bundler.go @@ -1929,7 +1929,7 @@ func (s *scanner) processScannedFiles(entryPointMeta []graph.EntryPoint) []scann s.log.AddErrorWithNotes(&tracker, record.Range, fmt.Sprintf("The file %q was loaded with the %q loader", otherFile.inputFile.Source.PrettyPath, config.LoaderToString[otherFile.inputFile.Loader]), []logger.MsgData{ - tracker.MsgData(js_lexer.RangeOfImportAssertion(result.file.inputFile.Source, *ast.FindAssertion(*record.Assertions, "type")), + tracker.MsgData(js_lexer.RangeOfImportAssertion(result.file.inputFile.Source, *ast.FindAssertion(record.Assertions.Entries, "type")), "This import assertion requires the loader to be \"json\" instead:"), {Text: "You need to either reconfigure esbuild to ensure that the loader for this file is \"json\" or you need to remove this import assertion."}}) } diff --git a/internal/bundler_tests/bundler_default_test.go b/internal/bundler_tests/bundler_default_test.go index 30d14f82be8..3548cef8747 100644 --- a/internal/bundler_tests/bundler_default_test.go +++ b/internal/bundler_tests/bundler_default_test.go @@ -7560,6 +7560,17 @@ func TestCommentPreservation(t *testing.T) { import('foo' /* after */), ) + console.log( + import('foo', /* before */ { assert: { type: 'json' } }), + import('foo', { /* before */ assert: { type: 'json' } }), + import('foo', { assert: /* before */ { type: 'json' } }), + import('foo', { assert: { /* before */ type: 'json' } }), + import('foo', { assert: { type: /* before */ 'json' } }), + import('foo', { assert: { type: 'json' /* before */ } }), + import('foo', { assert: { type: 'json' } /* before */ }), + import('foo', { assert: { type: 'json' } } /* before */), + ) + console.log( require(/* before */ foo), require(/* before */ 'foo'), @@ -7689,6 +7700,30 @@ func TestCommentPreservation(t *testing.T) { }) } +func TestCommentPreservationImportAssertions(t *testing.T) { + default_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.jsx": ` + import 'foo' /* before */ assert { type: 'json' } + import 'foo' assert /* before */ { type: 'json' } + import 'foo' assert { /* before */ type: 'json' } + import 'foo' assert { type: /* before */ 'json' } + import 'foo' assert { type: 'json' /* before */ } + `, + }, + entryPaths: []string{"/entry.jsx"}, + options: config.Options{ + Mode: config.ModeBundle, + AbsOutputDir: "/out", + ExternalSettings: config.ExternalSettings{ + PreResolve: config.ExternalMatchers{ + Exact: map[string]bool{"foo": true}, + }, + }, + }, + }) +} + func TestCommentPreservationTransformJSX(t *testing.T) { default_suite.expectBundled(t, bundled{ files: map[string]string{ diff --git a/internal/bundler_tests/snapshots/snapshots_default.txt b/internal/bundler_tests/snapshots/snapshots_default.txt index ec72857eacf..4208d97557e 100644 --- a/internal/bundler_tests/snapshots/snapshots_default.txt +++ b/internal/bundler_tests/snapshots/snapshots_default.txt @@ -320,6 +320,44 @@ console.log( /* after */ ) ); +console.log( + import( + "foo", + /* before */ + { assert: { type: "json" } } + ), + import("foo", { + /* before */ + assert: { type: "json" } + }), + import("foo", { + assert: + /* before */ + { type: "json" } + }), + import("foo", { assert: { + /* before */ + type: "json" + } }), + import("foo", { assert: { + type: + /* before */ + "json" + } }), + import("foo", { assert: { + type: "json" + /* before */ + } }), + import("foo", { + assert: { type: "json" } + /* before */ + }), + import( + "foo", + { assert: { type: "json" } } + /* before */ + ) +); console.log( require( /* before */ @@ -559,6 +597,26 @@ console.log( a ? b : c ); +================================================================================ +TestCommentPreservationImportAssertions +---------- /out/entry.js ---------- +// entry.jsx +import "foo" assert { type: "json" }; +import "foo" assert { type: "json" }; +import "foo" assert { + /* before */ + type: "json" +}; +import "foo" assert { + type: + /* before */ + "json" +}; +import "foo" assert { + type: "json" + /* before */ +}; + ================================================================================ TestCommentPreservationPreserveJSX ---------- /out/entry.js ---------- diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go index c677b2e8b03..b22af639cc1 100644 --- a/internal/js_parser/js_parser.go +++ b/internal/js_parser/js_parser.go @@ -1858,7 +1858,7 @@ func (p *parser) checkForLegacyOctalLiteral(e js_ast.E) { func (p *parser) notesForAssertTypeJSON(record *ast.ImportRecord, alias string) []logger.MsgData { return []logger.MsgData{p.tracker.MsgData( - js_lexer.RangeOfImportAssertion(p.source, *ast.FindAssertion(*record.Assertions, "type")), + js_lexer.RangeOfImportAssertion(p.source, *ast.FindAssertion(record.Assertions.Entries, "type")), "This is considered an import of a standard JSON module because of the import assertion here:"), {Text: fmt.Sprintf("You can either keep the import assertion and only use the \"default\" import, "+ "or you can remove the import assertion and use the %q import (which is non-standard behavior).", alias)}} @@ -5951,7 +5951,7 @@ func (p *parser) parseLabelName() *js_ast.LocRef { return &name } -func (p *parser) parsePath() (logger.Loc, string, *[]ast.AssertEntry, ast.ImportRecordFlags) { +func (p *parser) parsePath() (logger.Loc, string, *ast.ImportAssertions, ast.ImportRecordFlags) { var flags ast.ImportRecordFlags pathLoc := p.lexer.Loc() pathText := helpers.UTF16ToString(p.lexer.StringLiteral()) @@ -5962,17 +5962,19 @@ func (p *parser) parsePath() (logger.Loc, string, *[]ast.AssertEntry, ast.Import } // See https://github.com/tc39/proposal-import-assertions for more info - var assertions *[]ast.AssertEntry + var assertions *ast.ImportAssertions if !p.lexer.HasNewlineBefore && p.lexer.IsContextualKeyword("assert") { // "import './foo.json' assert { type: 'json' }" var entries []ast.AssertEntry duplicates := make(map[string]logger.Range) + assertLoc := p.saveExprCommentsHere() p.lexer.Next() + openBraceLoc := p.saveExprCommentsHere() p.lexer.Expect(js_lexer.TOpenBrace) for p.lexer.Token != js_lexer.TCloseBrace { // Parse the key - keyLoc := p.lexer.Loc() + keyLoc := p.saveExprCommentsHere() preferQuotedKey := false var key []uint16 var keyText string @@ -5995,7 +5997,7 @@ func (p *parser) parsePath() (logger.Loc, string, *[]ast.AssertEntry, ast.Import p.lexer.Expect(js_lexer.TColon) // Parse the value - valueLoc := p.lexer.Loc() + valueLoc := p.saveExprCommentsHere() value := p.lexer.StringLiteral() p.lexer.Expect(js_lexer.TStringLiteral) @@ -6018,8 +6020,14 @@ func (p *parser) parsePath() (logger.Loc, string, *[]ast.AssertEntry, ast.Import p.lexer.Next() } + closeBraceLoc := p.saveExprCommentsHere() p.lexer.Expect(js_lexer.TCloseBrace) - assertions = &entries + assertions = &ast.ImportAssertions{ + Entries: entries, + AssertLoc: assertLoc, + InnerOpenBraceLoc: openBraceLoc, + InnerCloseBraceLoc: closeBraceLoc, + } } return pathLoc, pathText, assertions, flags @@ -6413,7 +6421,7 @@ func (p *parser) parseStmt(opts parseStmtOpts) js_ast.Stmt { var alias *js_ast.ExportStarAlias var pathLoc logger.Loc var pathText string - var assertions *[]ast.AssertEntry + var assertions *ast.ImportAssertions var flags ast.ImportRecordFlags if p.lexer.IsContextualKeyword("as") { @@ -7359,7 +7367,7 @@ func extractDeclsForBinding(binding js_ast.Binding, decls []js_ast.Decl) []js_as return decls } -func (p *parser) addImportRecord(kind ast.ImportKind, loc logger.Loc, text string, assertions *[]ast.AssertEntry, flags ast.ImportRecordFlags) uint32 { +func (p *parser) addImportRecord(kind ast.ImportKind, loc logger.Loc, text string, assertions *ast.ImportAssertions, flags ast.ImportRecordFlags) uint32 { index := uint32(len(p.importRecords)) p.importRecords = append(p.importRecords, ast.ImportRecord{ Kind: kind, @@ -13540,7 +13548,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO isThenCatchTarget := e == p.thenCatchChain.nextTarget && p.thenCatchChain.hasCatch e.Expr = p.visitExpr(e.Expr) - var assertions *[]ast.AssertEntry + var assertions *ast.ImportAssertions var flags ast.ImportRecordFlags if e.OptionsOrNil.Data != nil { e.OptionsOrNil = p.visitExpr(e.OptionsOrNil) @@ -13593,7 +13601,14 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO break } if entries != nil { - assertions = &entries + assertions = &ast.ImportAssertions{ + Entries: entries, + AssertLoc: prop.Key.Loc, + InnerOpenBraceLoc: prop.ValueOrNil.Loc, + InnerCloseBraceLoc: value.CloseBraceLoc, + OuterOpenBraceLoc: e.OptionsOrNil.Loc, + OuterCloseBraceLoc: object.CloseBraceLoc, + } why = "" } } else { diff --git a/internal/js_printer/js_printer.go b/internal/js_printer/js_printer.go index 76ccf5964b7..2b3f2e9f075 100644 --- a/internal/js_printer/js_printer.go +++ b/internal/js_printer/js_printer.go @@ -1267,7 +1267,12 @@ func (p *printer) printRequireOrImportExpr(importRecordIndex uint32, level js_as p.print("(") } - isMultiLine := p.willPrintExprCommentsAtLoc(record.Range.Loc) || p.willPrintExprCommentsAtLoc(closeParenLoc) + isMultiLine := p.willPrintExprCommentsAtLoc(record.Range.Loc) || + p.willPrintExprCommentsAtLoc(closeParenLoc) || + (record.Assertions != nil && + !p.options.UnsupportedFeatures.Has(compat.DynamicImport) && + !p.options.UnsupportedFeatures.Has(compat.ImportAssertions) && + p.willPrintExprCommentsAtLoc(record.Assertions.OuterOpenBraceLoc)) if isMultiLine { p.printNewline() p.options.Indent++ @@ -1276,7 +1281,7 @@ func (p *printer) printRequireOrImportExpr(importRecordIndex uint32, level js_as p.printExprCommentsAtLoc(record.Range.Loc) p.printPath(importRecordIndex, kind) if !p.options.UnsupportedFeatures.Has(compat.DynamicImport) { - p.printImportCallAssertions(record.Assertions) + p.printImportCallAssertions(record.Assertions, isMultiLine) } if isMultiLine { p.printNewline() @@ -3382,40 +3387,104 @@ func (p *printer) printPath(importRecordIndex uint32, importKind ast.ImportKind) if record.Assertions != nil && importKind == ast.ImportStmt { p.printSpace() + p.addSourceMapping(record.Assertions.AssertLoc) p.print("assert") p.printSpace() p.printImportAssertionsClause(*record.Assertions) } } -func (p *printer) printImportCallAssertions(assertions *[]ast.AssertEntry) { +func (p *printer) printImportCallAssertions(assertions *ast.ImportAssertions, outerIsMultiLine bool) { // Just omit import assertions if they aren't supported if p.options.UnsupportedFeatures.Has(compat.ImportAssertions) { return } - if assertions != nil { - p.print(",") + if assertions == nil { + return + } + + isMultiLine := p.willPrintExprCommentsAtLoc(assertions.AssertLoc) || + p.willPrintExprCommentsAtLoc(assertions.InnerOpenBraceLoc) || + p.willPrintExprCommentsAtLoc(assertions.OuterCloseBraceLoc) + + p.print(",") + if outerIsMultiLine { + p.printNewline() + p.printIndent() + } else { p.printSpace() - p.print("{") + } + p.printExprCommentsAtLoc(assertions.OuterOpenBraceLoc) + p.addSourceMapping(assertions.OuterOpenBraceLoc) + p.print("{") + + if isMultiLine { + p.printNewline() + p.options.Indent++ + p.printIndent() + } else { p.printSpace() - p.print("assert:") + } + + p.printExprCommentsAtLoc(assertions.AssertLoc) + p.addSourceMapping(assertions.AssertLoc) + p.print("assert:") + + if p.willPrintExprCommentsAtLoc(assertions.InnerOpenBraceLoc) { + p.printNewline() + p.options.Indent++ + p.printIndent() + p.printExprCommentsAtLoc(assertions.InnerOpenBraceLoc) + p.printImportAssertionsClause(*assertions) + p.options.Indent-- + } else { p.printSpace() p.printImportAssertionsClause(*assertions) + } + + if isMultiLine { + p.printNewline() + p.printExprCommentsAfterCloseTokenAtLoc(assertions.OuterCloseBraceLoc) + p.options.Indent-- + p.printIndent() + } else { p.printSpace() - p.print("}") } + + p.addSourceMapping(assertions.OuterCloseBraceLoc) + p.print("}") } -func (p *printer) printImportAssertionsClause(assertions []ast.AssertEntry) { +func (p *printer) printImportAssertionsClause(assertions ast.ImportAssertions) { + isMultiLine := p.willPrintExprCommentsAtLoc(assertions.InnerCloseBraceLoc) + if !isMultiLine { + for _, entry := range assertions.Entries { + if p.willPrintExprCommentsAtLoc(entry.KeyLoc) || p.willPrintExprCommentsAtLoc(entry.ValueLoc) { + isMultiLine = true + break + } + } + } + + p.addSourceMapping(assertions.InnerOpenBraceLoc) p.print("{") + if isMultiLine { + p.options.Indent++ + } - for i, entry := range assertions { + for i, entry := range assertions.Entries { if i > 0 { p.print(",") } + if isMultiLine { + p.printNewline() + p.printIndent() + } else { + p.printSpace() + } - p.printSpace() + p.printExprCommentsAtLoc(entry.KeyLoc) p.addSourceMapping(entry.KeyLoc) if !entry.PreferQuotedKey && p.canPrintIdentifierUTF16(entry.Key) { p.printSpaceBeforeIdentifier() @@ -3425,15 +3494,32 @@ func (p *printer) printImportAssertionsClause(assertions []ast.AssertEntry) { } p.print(":") - p.printSpace() - p.addSourceMapping(entry.ValueLoc) - p.printQuotedUTF16(entry.Value, false /* allowBacktick */) + if p.willPrintExprCommentsAtLoc(entry.ValueLoc) { + p.printNewline() + p.options.Indent++ + p.printIndent() + p.printExprCommentsAtLoc(entry.ValueLoc) + p.addSourceMapping(entry.ValueLoc) + p.printQuotedUTF16(entry.Value, false /* allowBacktick */) + p.options.Indent-- + } else { + p.printSpace() + p.addSourceMapping(entry.ValueLoc) + p.printQuotedUTF16(entry.Value, false /* allowBacktick */) + } } - if len(assertions) > 0 { + if isMultiLine { + p.printNewline() + p.printExprCommentsAfterCloseTokenAtLoc(assertions.InnerCloseBraceLoc) + p.options.Indent-- + p.printIndent() + } else if len(assertions.Entries) > 0 { p.printSpace() } + + p.addSourceMapping(assertions.InnerCloseBraceLoc) p.print("}") }