fix #2390: transform w/ external legal comments

evanw · Dec 19, 2022 · 1ec8085 · 1ec8085
1 parent 6a73c5e
commit 1ec8085
Show file tree

Hide file tree

Showing 8 changed files with 82 additions and 36 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -48,6 +48,28 @@
 
     Note that you can still customize this behavior with the `--legal-comments=` flag. For example, you can use `--legal-comments=none` to turn this off, or you can use `--legal-comments=linked` to put these comments in a separate `.LEGAL.txt` file instead.
 
+* Enable `external` legal comments with the transform API ([#2390](https://github.com/evanw/esbuild/issues/2390))
+
+    Previously esbuild's transform API only supported `none`, `inline`, or `eof` legal comments. With this release, `external` legal comments are now also supported with the transform API. This only applies to the JS and Go APIs, not to the CLI, and looks like this:
+
+    * JS:
+
+        ```js
+        const { code, legalComments } = await esbuild.transform(input, {
+          legalComments: 'external',
+        })
+        ```
+
+    * Go:
+
+        ```go
+        result := api.Transform(input, api.TransformOptions{
+          LegalComments: api.LegalCommentsEndOfFile,
+        })
+        code := result.Code
+        legalComments := result.LegalComments
+        ```
+
 * Fix duplicate function declaration edge cases ([#2757](https://github.com/evanw/esbuild/issues/2757))
 
     The change in the previous release to forbid duplicate function declarations in certain cases accidentally forbid some edge cases that should have been allowed. Specifically duplicate function declarations are forbidden in nested blocks in strict mode and at the top level of modules, but are allowed when they are declared at the top level of function bodies. This release fixes the regression by re-allowing the last case.

diff --git a/cmd/esbuild/service.go b/cmd/esbuild/service.go
@@ -993,6 +993,10 @@ func (service *serviceType) handleTransformRequest(id uint32, request map[string
 		"map":   string(result.Map),
 	}
 
+	if result.LegalComments != nil {
+		response["legalComments"] = string(result.LegalComments)
+	}
+
 	if result.MangleCache != nil {
 		response["mangleCache"] = result.MangleCache
 	}

diff --git a/lib/shared/common.ts b/lib/shared/common.ts
@@ -737,6 +737,7 @@ export function createChannel(streamIn: StreamIn): StreamOut {
           let next = () => {
             if (--outstanding === 0) {
               let result: types.TransformResult = { warnings, code: response!.code, map: response!.map }
+              if ('legalComments' in response!) result.legalComments = response?.legalComments
               if (response!.mangleCache) result.mangleCache = response?.mangleCache
               callback(null, result)
             }

diff --git a/lib/shared/stdio_protocol.ts b/lib/shared/stdio_protocol.ts
@@ -115,6 +115,7 @@ export interface TransformResponse {
   map: string;
   mapFS: boolean;
 
+  legalComments?: string;
   mangleCache?: Record<string, string | false>;
 }
 

diff --git a/lib/shared/types.ts b/lib/shared/types.ts
@@ -280,6 +280,8 @@ export interface TransformResult {
   warnings: Message[];
   /** Only when "mangleCache" is present */
   mangleCache?: Record<string, string | false>;
+  /** Only when "legalComments" is "external" */
+  legalComments?: string;
 }
 
 export interface TransformFailure extends Error {

diff --git a/pkg/api/api.go b/pkg/api/api.go
@@ -432,8 +432,9 @@ type TransformResult struct {
 	Errors   []Message
 	Warnings []Message
 
-	Code []byte
-	Map  []byte
+	Code          []byte
+	Map           []byte
+	LegalComments []byte
 
 	MangleCache map[string]interface{}
 }

diff --git a/pkg/api/api_impl.go b/pkg/api/api_impl.go
@@ -1556,8 +1556,12 @@ func transformImpl(input string, transformOpts TransformOptions) TransformResult
 		log.AddError(nil, logger.Range{},
 			"Must use \"sourcefile\" with \"sourcemap\" to set the original file name")
 	}
-	if options.LegalComments.HasExternalFile() {
-		log.AddError(nil, logger.Range{}, "Cannot transform with linked or external legal comments")
+	if logger.API == logger.CLIAPI {
+		if options.LegalComments.HasExternalFile() {
+			log.AddError(nil, logger.Range{}, "Cannot transform with linked or external legal comments")
+		}
+	} else if options.LegalComments == config.LegalCommentsLinkedWithComment {
+		log.AddError(nil, logger.Range{}, "Cannot transform with linked legal comments")
 	}
 
 	// Set the output mode using other settings
@@ -1591,16 +1595,24 @@ func transformImpl(input string, transformOpts TransformOptions) TransformResult
 	// Return the results
 	var code []byte
 	var sourceMap []byte
+	var legalComments []byte
+
+	var shortestAbsPath string
+	for _, result := range results {
+		if shortestAbsPath == "" || len(result.AbsPath) < len(shortestAbsPath) {
+			shortestAbsPath = result.AbsPath
+		}
+	}
 
-	// Unpack the JavaScript file and the source map file
-	if len(results) == 1 {
-		code = results[0].Contents
-	} else if len(results) == 2 {
-		a, b := results[0], results[1]
-		if a.AbsPath == b.AbsPath+".map" {
-			sourceMap, code = a.Contents, b.Contents
-		} else if a.AbsPath+".map" == b.AbsPath {
-			code, sourceMap = a.Contents, b.Contents
+	// Unpack the JavaScript file, the source map file, and the legal comments file
+	for _, result := range results {
+		switch result.AbsPath {
+		case shortestAbsPath:
+			code = result.Contents
+		case shortestAbsPath + ".map":
+			sourceMap = result.Contents
+		case shortestAbsPath + ".LEGAL.txt":
+			legalComments = result.Contents
 		}
 	}
 
@@ -1611,11 +1623,12 @@ func transformImpl(input string, transformOpts TransformOptions) TransformResult
 
 	msgs := log.Done()
 	return TransformResult{
-		Errors:      convertMessagesToPublic(logger.Error, msgs),
-		Warnings:    convertMessagesToPublic(logger.Warning, msgs),
-		Code:        code,
-		Map:         sourceMap,
-		MangleCache: mangleCache,
+		Errors:        convertMessagesToPublic(logger.Error, msgs),
+		Warnings:      convertMessagesToPublic(logger.Warning, msgs),
+		Code:          code,
+		Map:           sourceMap,
+		LegalComments: legalComments,
+		MangleCache:   mangleCache,
 	}
 }
 

diff --git a/scripts/js-api-tests.js b/scripts/js-api-tests.js
@@ -5040,39 +5040,41 @@ let transformTests = {
   async transformLegalCommentsJS({ esbuild }) {
     assert.strictEqual((await esbuild.transform(`//!x\ny()`, { legalComments: 'none' })).code, `y();\n`)
     assert.strictEqual((await esbuild.transform(`//!x\ny()`, { legalComments: 'inline' })).code, `//!x\ny();\n`)
-    assert.strictEqual((await esbuild.transform(`//!x\ny()`, { legalComments: 'eof' })).code, `y();\n//!x\n`)
+
+    const eofResult = await esbuild.transform(`//!x\ny()`, { legalComments: 'eof' })
+    assert.strictEqual(eofResult.code, `y();\n//!x\n`)
+    assert.strictEqual(eofResult.legalComments, undefined)
+
+    const externalResult = await esbuild.transform(`//!x\ny()`, { legalComments: 'external' })
+    assert.strictEqual(externalResult.code, `y();\n`)
+    assert.strictEqual(externalResult.legalComments, `//!x\n`)
+
     try {
       await esbuild.transform(``, { legalComments: 'linked' })
       throw new Error('Expected a transform failure')
     } catch (e) {
-      if (!e || !e.errors || !e.errors[0] || e.errors[0].text !== 'Cannot transform with linked or external legal comments')
-        throw e
-    }
-    try {
-      await esbuild.transform(``, { legalComments: 'external' })
-      throw new Error('Expected a transform failure')
-    } catch (e) {
-      if (!e || !e.errors || !e.errors[0] || e.errors[0].text !== 'Cannot transform with linked or external legal comments')
+      if (!e || !e.errors || !e.errors[0] || e.errors[0].text !== 'Cannot transform with linked legal comments')
         throw e
     }
   },
 
   async transformLegalCommentsCSS({ esbuild }) {
     assert.strictEqual((await esbuild.transform(`/*!x*/\ny{}`, { loader: 'css', legalComments: 'none' })).code, `y {\n}\n`)
     assert.strictEqual((await esbuild.transform(`/*!x*/\ny{}`, { loader: 'css', legalComments: 'inline' })).code, `/*!x*/\ny {\n}\n`)
-    assert.strictEqual((await esbuild.transform(`/*!x*/\ny{}`, { loader: 'css', legalComments: 'eof' })).code, `y {\n}\n/*!x*/\n`)
+
+    const eofResult = await esbuild.transform(`/*!x*/\ny{}`, { loader: 'css', legalComments: 'eof' })
+    assert.strictEqual(eofResult.code, `y {\n}\n/*!x*/\n`)
+    assert.strictEqual(eofResult.legalComments, undefined)
+
+    const externalResult = await esbuild.transform(`/*!x*/\ny{}`, { loader: 'css', legalComments: 'external' })
+    assert.strictEqual(externalResult.code, `y {\n}\n`)
+    assert.strictEqual(externalResult.legalComments, `/*!x*/\n`)
+
     try {
       await esbuild.transform(``, { legalComments: 'linked' })
       throw new Error('Expected a transform failure')
     } catch (e) {
-      if (!e || !e.errors || !e.errors[0] || e.errors[0].text !== 'Cannot transform with linked or external legal comments')
-        throw e
-    }
-    try {
-      await esbuild.transform(``, { legalComments: 'external' })
-      throw new Error('Expected a transform failure')
-    } catch (e) {
-      if (!e || !e.errors || !e.errors[0] || e.errors[0].text !== 'Cannot transform with linked or external legal comments')
+      if (!e || !e.errors || !e.errors[0] || e.errors[0].text !== 'Cannot transform with linked legal comments')
         throw e
     }
   },