Skip to content

Commit

Permalink
fix #2745: another change to legal comments
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Dec 19, 2022
1 parent da0c253 commit 47dd4de
Show file tree
Hide file tree
Showing 8 changed files with 423 additions and 44 deletions.
48 changes: 48 additions & 0 deletions CHANGELOG.md
@@ -1,5 +1,53 @@
# Changelog

## Unreleased

* Change the default "legal comment" behavior again ([#2745](https://github.com/evanw/esbuild/issues/2745))

The legal comments feature automatically gathers comments containing `@license` or `@preserve` and puts the comments somewhere (either in the generated code or in a separate file). This behavior used to be on by default but was disabled by default in version 0.16.0 because automatically inserting comments is potentially confusing and misleading. These comments can appear to be assigning the copyright of your code to another entity. And this behavior can be especially problematic if it happens automatically by default since you may not even be aware of it happening. For example, if you bundle the TypeScript compiler the preserving legal comments means your source code would contain this comment, which appears to be assigning the copyright of all of your code to Microsoft:

```js
/*! *****************************************************************************
Copyright (c) Microsoft Corporation. All rights reserved.
Licensed under the Apache License, Version 2.0 (the "License"); you may not use
this file except in compliance with the License. You may obtain a copy of the
License at http://www.apache.org/licenses/LICENSE-2.0
THIS CODE IS PROVIDED ON AN *AS IS* BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, EITHER EXPRESS OR IMPLIED, INCLUDING WITHOUT LIMITATION ANY IMPLIED
WARRANTIES OR CONDITIONS OF TITLE, FITNESS FOR A PARTICULAR PURPOSE,
MERCHANTABLITY OR NON-INFRINGEMENT.
See the Apache Version 2.0 License for specific language governing permissions
and limitations under the License.
***************************************************************************** */
```

However, people have asked for this feature to be re-enabled by default. To resolve the confusion about what these comments are applying to, esbuild's default behavior will now be to attempt to describe which package the comments are coming from. So while this feature has been re-enabled by default, the output will now look something like this instead:

```js
/*! Bundled license information:
typescript/lib/typescript.js:
(*! *****************************************************************************
Copyright (c) Microsoft Corporation. All rights reserved.
Licensed under the Apache License, Version 2.0 (the "License"); you may not use
this file except in compliance with the License. You may obtain a copy of the
License at http://www.apache.org/licenses/LICENSE-2.0
THIS CODE IS PROVIDED ON AN *AS IS* BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, EITHER EXPRESS OR IMPLIED, INCLUDING WITHOUT LIMITATION ANY IMPLIED
WARRANTIES OR CONDITIONS OF TITLE, FITNESS FOR A PARTICULAR PURPOSE,
MERCHANTABLITY OR NON-INFRINGEMENT.
See the Apache Version 2.0 License for specific language governing permissions
and limitations under the License.
***************************************************************************** *)
*/
```

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.

## 0.16.9

* Update to Unicode 15.0.0
Expand Down
144 changes: 144 additions & 0 deletions internal/bundler_tests/bundler_default_test.go
Expand Up @@ -3477,6 +3477,150 @@ func TestLegalCommentsAvoidSlashTagExternal(t *testing.T) {
})
}

func TestLegalCommentsManyEndOfFile(t *testing.T) {
default_suite.expectBundled(t, bundled{
files: map[string]string{
"/project/entry.js": `
import './a'
import './b'
import './c'
import 'some-pkg/js'
`,
"/project/a.js": `console.log('in a') //! Copyright notice 1`,
"/project/b.js": `console.log('in b') //! Copyright notice 1`,
"/project/c.js": `
function foo() {
/*
* @license
* Copyright notice 2
*/
console.log('in c')
// @preserve This is another comment
}
foo()
`,
"/project/node_modules/some-pkg/js/index.js": `import "some-other-pkg/js" //! (c) Good Software Corp`,
"/project/node_modules/some-other-pkg/js/index.js": `
function bar() {
/*
* @preserve
* (c) Evil Software Corp
*/
console.log('some-other-pkg')
}
bar()
`,

"/project/entry.css": `
@import "./a.css";
@import "./b.css";
@import "./c.css";
@import 'some-pkg/css';
`,
"/project/a.css": `a { zoom: 2 } /*! Copyright notice 1 */`,
"/project/b.css": `b { zoom: 2 } /*! Copyright notice 1 */`,
"/project/c.css": `
/*
* @license
* Copyright notice 2
*/
c {
zoom: 2
}
/* @preserve This is another comment */
`,
"/project/node_modules/some-pkg/css/index.css": `@import "some-other-pkg/css"; /*! (c) Good Software Corp */`,
"/project/node_modules/some-other-pkg/css/index.css": `
.some-other-pkg {
zoom: 2
}
/** @preserve
* (c) Evil Software Corp
*/
`,
},
entryPaths: []string{"/project/entry.js", "/project/entry.css"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputDir: "/out",
MinifyWhitespace: true,
LegalComments: config.LegalCommentsEndOfFile,
},
})
}

func TestLegalCommentsManyLinked(t *testing.T) {
default_suite.expectBundled(t, bundled{
files: map[string]string{
"/project/entry.js": `
import './a'
import './b'
import './c'
import 'some-pkg/js'
`,
"/project/a.js": `console.log('in a') //! Copyright notice 1`,
"/project/b.js": `console.log('in b') //! Copyright notice 1`,
"/project/c.js": `
function foo() {
/*
* @license
* Copyright notice 2
*/
console.log('in c')
// @preserve This is another comment
}
foo()
`,
"/project/node_modules/some-pkg/js/index.js": `import "some-other-pkg/js" //! (c) Good Software Corp`,
"/project/node_modules/some-other-pkg/js/index.js": `
function bar() {
/*
* @preserve
* (c) Evil Software Corp
*/
console.log('some-other-pkg')
}
bar()
`,

"/project/entry.css": `
@import "./a.css";
@import "./b.css";
@import "./c.css";
@import 'some-pkg/css';
`,
"/project/a.css": `a { zoom: 2 } /*! Copyright notice 1 */`,
"/project/b.css": `b { zoom: 2 } /*! Copyright notice 1 */`,
"/project/c.css": `
/*
* @license
* Copyright notice 2
*/
c {
zoom: 2
}
/* @preserve This is another comment */
`,
"/project/node_modules/some-pkg/css/index.css": `@import "some-other-pkg/css"; /*! (c) Good Software Corp */`,
"/project/node_modules/some-other-pkg/css/index.css": `
.some-other-pkg {
zoom: 2
}
/** @preserve
* (c) Evil Software Corp
*/
`,
},
entryPaths: []string{"/project/entry.js", "/project/entry.css"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputDir: "/out",
MinifyWhitespace: true,
LegalComments: config.LegalCommentsLinkedWithComment,
},
})
}

// The IIFE should not be an arrow function when targeting ES5
func TestIIFE_ES5(t *testing.T) {
default_suite.expectBundled(t, bundled{
Expand Down
88 changes: 88 additions & 0 deletions internal/bundler_tests/snapshots/snapshots_default.txt
Expand Up @@ -2096,6 +2096,94 @@ c {
/* entry.css */
/*! For license information please see entry.css.LEGAL.txt */

================================================================================
TestLegalCommentsManyEndOfFile
---------- /out/entry.js ----------
console.log("in a");console.log("in b");function foo(){console.log("in c");}foo();function bar(){console.log("some-other-pkg")}bar();
//! Copyright notice 1
/*
* @license
* Copyright notice 2
*/
// @preserve This is another comment
/*! Bundled license information:

some-other-pkg/js/index.js:
(*
* @preserve
* (c) Evil Software Corp
*)

some-pkg/js/index.js:
(*! (c) Good Software Corp *)
*/

---------- /out/entry.css ----------
a{zoom:2}b{zoom:2}c{zoom:2}.some-other-pkg{zoom:2}
/*! Copyright notice 1 */
/*
* @license
* Copyright notice 2
*/
/* @preserve This is another comment */
/*! Bundled license information:

some-other-pkg/css/index.css:
(** @preserve
* (c) Evil Software Corp
*)

some-pkg/css/index.css:
(*! (c) Good Software Corp *)
*/

================================================================================
TestLegalCommentsManyLinked
---------- /out/entry.js.LEGAL.txt ----------
//! Copyright notice 1
/*
* @license
* Copyright notice 2
*/
// @preserve This is another comment

Bundled license information:

some-other-pkg/js/index.js:
/*
* @preserve
* (c) Evil Software Corp
*/

some-pkg/js/index.js:
//! (c) Good Software Corp

---------- /out/entry.js ----------
console.log("in a");console.log("in b");function foo(){console.log("in c");}foo();function bar(){console.log("some-other-pkg")}bar();
/*! For license information please see entry.js.LEGAL.txt */

---------- /out/entry.css.LEGAL.txt ----------
/*! Copyright notice 1 */
/*
* @license
* Copyright notice 2
*/
/* @preserve This is another comment */

Bundled license information:

some-other-pkg/css/index.css:
/** @preserve
* (c) Evil Software Corp
*/

some-pkg/css/index.css:
/*! (c) Good Software Corp */

---------- /out/entry.css ----------
a{zoom:2}b{zoom:2}c{zoom:2}.some-other-pkg{zoom:2}
/*! For license information please see entry.css.LEGAL.txt */

================================================================================
TestLegalCommentsModifyIndent
---------- /out/entry.js ----------
Expand Down
9 changes: 3 additions & 6 deletions internal/css_printer/css_printer.go
Expand Up @@ -20,7 +20,7 @@ type printer struct {
options Options
importRecords []ast.ImportRecord
css []byte
extractedLegalComments map[string]bool
extractedLegalComments []string
jsonMetadataImports []string
builder sourcemap.ChunkBuilder
}
Expand All @@ -45,7 +45,7 @@ type Options struct {

type PrintResult struct {
CSS []byte
ExtractedLegalComments map[string]bool
ExtractedLegalComments []string
JSONMetadataImports []string

// This source map chunk just contains the VLQ-encoded offsets for the "CSS"
Expand Down Expand Up @@ -100,10 +100,7 @@ func (p *printer) printRule(rule css_ast.Rule, indent int32, omitTrailingSemicol
case config.LegalCommentsEndOfFile,
config.LegalCommentsLinkedWithComment,
config.LegalCommentsExternalWithoutComment:
if p.extractedLegalComments == nil {
p.extractedLegalComments = make(map[string]bool)
}
p.extractedLegalComments[r.Text] = true
p.extractedLegalComments = append(p.extractedLegalComments, r.Text)
return
}
}
Expand Down
4 changes: 4 additions & 0 deletions internal/js_parser/js_parser_test.go
Expand Up @@ -4625,13 +4625,17 @@ func TestPreservedComments(t *testing.T) {
expectPrinted(t, "//!", "//!\n")
expectPrinted(t, "//@license", "//@license\n")
expectPrinted(t, "//@preserve", "//@preserve\n")
expectPrinted(t, "// @license", "// @license\n")
expectPrinted(t, "// @preserve", "// @preserve\n")

expectPrinted(t, "/**/", "")
expectPrinted(t, "/*preserve*/", "")
expectPrinted(t, "/*@__PURE__*/", "")
expectPrinted(t, "/*!*/", "/*!*/\n")
expectPrinted(t, "/*@license*/", "/*@license*/\n")
expectPrinted(t, "/*@preserve*/", "/*@preserve*/\n")
expectPrinted(t, "/*\n * @license\n */", "/*\n * @license\n */\n")
expectPrinted(t, "/*\n * @preserve\n */", "/*\n * @preserve\n */\n")

expectPrinted(t, "foo() //! test", "foo();\n//! test\n")
expectPrinted(t, "//! test\nfoo()", "//! test\nfoo();\n")
Expand Down
9 changes: 3 additions & 6 deletions internal/js_printer/js_printer.go
Expand Up @@ -334,7 +334,7 @@ type printer struct {
renamer renamer.Renamer
importRecords []ast.ImportRecord
callTarget js_ast.E
extractedLegalComments map[string]bool
extractedLegalComments []string
js []byte
jsonMetadataImports []string
options Options
Expand Down Expand Up @@ -3203,10 +3203,7 @@ func (p *printer) printStmt(stmt js_ast.Stmt, flags printStmtFlags) {
case config.LegalCommentsEndOfFile,
config.LegalCommentsLinkedWithComment,
config.LegalCommentsExternalWithoutComment:
if p.extractedLegalComments == nil {
p.extractedLegalComments = make(map[string]bool)
}
p.extractedLegalComments[text] = true
p.extractedLegalComments = append(p.extractedLegalComments, text)
return
}
}
Expand Down Expand Up @@ -3902,7 +3899,7 @@ type RequireOrImportMeta struct {

type PrintResult struct {
JS []byte
ExtractedLegalComments map[string]bool
ExtractedLegalComments []string
JSONMetadataImports []string

// This source map chunk just contains the VLQ-encoded offsets for the "JS"
Expand Down

0 comments on commit 47dd4de

Please sign in to comment.