evanw / esbuild

An extremely fast bundler for the web
https://esbuild.github.io/
MIT License
38.18k stars 1.15k forks source link

Risks of discarding legal comments by default #2745

Closed zarqman closed 1 year ago

zarqman commented 1 year ago

I'm wondering about the decision in 0.16.0 to start discarding legal comments by default.

The licenses for a large number of npms and other projects only allow redistribution (ie: publication on the web) if the license is somehow included. A condition to this effect is found in the widely used MIT and Apache 2 licenses, among others.

With this change, apps that are bundling for web use/distribution have high odds of accidentally forfeiting their legal right to distribute the bundled code unless they catch this change and take action to reenable legal comments.

Packages that are bundling for their own distribution often include a separate file with their licensing. However, when projects duplicate licensing info as code comments, it's likely because they are particularly sensitive to the legal notices not being lost. As such, they also likely want such notices included in their bundle so that any downstream rebundling properly includes them.

Between these two audiences, it seems to me that most of them will indeed need to turn legal notices back on and thus would be well served by having them then reenabled by default.

I do understand the rationale of avoiding the confusion caused by the previous default value of 'eof'.

What about changing the default to 'inline' instead? That would both avoid the prior confusion while also helping users of esbuild meet their legal obligations to use code bundled from dependencies.

tbjgolden commented 1 year ago

I've been looking over this:

The defacto standard has been to not remove comments with an exclamation at the start: /*! ... */, //! ..., or to remove comments containing: @license.

Only esbuild and swc do not try to preserve legal comments by default - perhaps it would make sense for both projects to enable it.

evanw commented 1 year ago

I realize that other tools do this, but it's really not a good way to distribute the licenses of your dependencies to your users. For example, if you bundle the TypeScript compiler and you include these comments, your code will be marked with this comment:

/*! *****************************************************************************
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.
***************************************************************************** */

This behavior is confusing because it looks like Microsoft is claiming copyright over all of the code you wrote. IMO these kinds of comments are still confusing even if they are placed inline with the minified code. It still looks like Microsoft (or whoever) owns the copyright to your code. I changed the default for this setting because I didn't want esbuild to be at fault for automatically stamping your code with the wrong license, which I guess is another kind of "risk" to consider.

tbjgolden commented 1 year ago

Would moving them to the bottom of the file by default be somewhat of a happy medium?

evanw commented 1 year ago

It sounds like you didn't read my comment? I'm saying that esbuild automatically including these comments in output files by default (regardless of where they are) is potentially confusing and misleading because these comments can appear to be assigning the copyright of your code to another entity. The fact that it happens automatically is problematic because people may not inspect the minified output and may not even be aware that this problem is happening.

tbjgolden commented 1 year ago

I did read the comment above, but (perhaps unfairly) read "didn't want esbuild to be at fault for automatically stamping your code with the wrong license" to mean "deliberately ignoring the precedent because I think it's silly".

I'm saying that not including these comments in output files by default is potentially leading users into unintended legal trouble when bundling their own code and others code together - after all, users of other minification tools could reasonably assume that legal comments would be preserved when using esbuild (I know I did).

Perhaps a non-technical solution might instead be to update the esbuild docs to makes it clear that "esbuild strips all comments including ones with attribution and licenses by default", or words to that effect?

zarqman commented 1 year ago

Here's what I think is being said:

This creates somewhat of a conundrum, as both positions are defensible. The only point of agreement is that a well-chosen default matters, as we cannot expect users to, by and large, be involved.

What about if, in EOF, Linked, and External modes, a 1 line "introduction" comment was added? It might be as simple as: /*! Bundled legal notices and comments follow: */ And then the various legal comments would follow. That lead-in line would provide some context that what follows is sourced from elsewhere and may not apply to the whole file. It could be expanded further, but I'm trying for something concise.

Additionally, when 2 or more notices are included, which I'd guess is rather common, the presence of multiple copyright notices should help clarify that any individual notice cannot apply to the whole file. But even if there's just one, the introductory comment should help.

Then, change the default to EOF so that people who don't check this setting are still covered properly.

Would the introductory comment line, along with a default of EOF, address everyone's concerns?

Additionally, for popular components whose output doesn't include the library name, Issues or PRs could be opened to add the name. (By anyone--not implying that burden falls on any participants here.)

evanw commented 1 year ago

Yeah that makes sense. I can try to rework the legal comments feature to present a more readable version of the bundled comments. It would probably also be helpful to try to say what packages the comments are from, or something to that effect.

evanw commented 1 year ago

Right now I'm thinking of going with something that looks like this at the end of the file:

/*! Bundled legal notices and comments:

json-graphql-server/lib/json-graphql-server.client.min.js:
  (*! For license information please see json-graphql-server.client.min.js.LICENSE.txt *)

decimal.js-light/decimal.js:
  (*! decimal.js-light v2.5.1 https://github.com/MikeMcl/decimal.js-light/LICENCE *)

classnames/index.js:
  (*!
    Copyright (c) 2018 Jed Watson.
    Licensed under the MIT License (MIT), see
    http://jedwatson.github.io/classnames
  *)

scheduler/cjs/scheduler.production.min.js:
  (** @license React v0.20.2
   * scheduler.production.min.js
   *
   * Copyright (c) Facebook, Inc. and its affiliates.
   *
   * This source code is licensed under the MIT license found in the
   * LICENSE file in the root directory of this source tree.
   *)

inflection/lib/inflection.js:
  (*!
   * inflection
   * Copyright(c) 2011 Ben Lin <ben@dreamerslab.com>
   * MIT Licensed
   *
   * @fileoverview
   * A port of inflection-js to node.js module.
   *)

react-dom/cjs/react-dom.production.min.js:
  (** @license React v17.0.2
   * react-dom.production.min.js
   *
   * Copyright (c) Facebook, Inc. and its affiliates.
   *
   * This source code is licensed under the MIT license found in the
   * LICENSE file in the root directory of this source tree.
   *)

object-assign/index.js:
  (*
  object-assign
  (c) Sindre Sorhus
  @license MIT
  *)

moment/moment.js:
  (*! moment.js *)
  (*! version : 2.29.4 *)
  (*! authors : Tim Wood, Iskren Chernev, Moment.js contributors *)
  (*! license : MIT *)
  (*! momentjs.com *)
*/
tbjgolden commented 1 year ago

Wow, it even plays nice with that unorthdox moment.js attribution comment style 🚀 nice work!

zarqman commented 1 year ago

Nice work, @evanw! Appreciate the thoughtful approach of identifying each source.