Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

minify should not remove licence comments #6677

Closed
tbjgolden opened this issue Dec 18, 2022 · 6 comments · Fixed by #6703
Closed

minify should not remove licence comments #6677

tbjgolden opened this issue Dec 18, 2022 · 6 comments · Fixed by #6703
Assignees
Labels
Milestone

Comments

@tbjgolden
Copy link

Describe the bug

Many users will be unaware that they could be breaking their software licence by having minify: true.

Other minifiers seem to use the convention that if a comment:

  • starts with a ! (e.g. /*! license ... */ or //! license ...), OR
  • looks like it contains a JSDoc licence /** @license */

that it should not be removed with the default minify settings.

Input code

/*! MIT @lorem example.org */

foo();

Config

{
  "jsc": {
    "parser": {
      "syntax": "ecmascript",
      "jsx": false
    },
    "target": "es5",
    "loose": false,
    "minify": {
      "compress": false,
      "mangle": false
    }
  },
  "module": {
    "type": "es6"
  },
  "minify": true,
  "isModule": true
}

Playground link

https://play.swc.rs/?version=1.3.23&code=H4sIAAAAAAAAA9PXUlTISM3JyVcozy%2FKSVHQ0ufiSsvP19C05gIAxeneNhsAAAA%3D&config=H4sIAAAAAAAAA0WMSwrDMAxE76K1t%2B0id8ghhKsEF%2F%2BQFIgxvnvlkpCdZubpdfiKh6VDRRbieUnLiicsQD6heA5VwRlm1YZRaDhQ5J10IvKyLZYidK0OUshha9PkS6pMIs%2BEeY83OUyUyueYRQdtlf7CN4zHoXzYW5D14mYeP5P5%2B3m0AAAA

Expected behavior

Comment is not removed

Actual behavior

Commit is removed

Version

1.3.23

Additional context

No response

@alexander-akait
Copy link
Collaborator

Looks like we don't remove them, we just don't print them, try to disable minify

@tbjgolden
Copy link
Author

Been doing some research:

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

  • terser, uglify, support this option behaviour by default
  • google closure compiler does not remove these comments
  • babel minify supports @license by default, but not !
  • esbuild has this option --legal-comments but has it disabled by default
  • swc has this option compress.format.comments: "some" but has it disabled by default

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/esbuild#2745

@ishowta
Copy link

ishowta commented Dec 20, 2022

When I specify an empty object for minify in playground "minify": {}, the comment disappears, but when I remove the minify field, the comment appears.

Also, when I run it locally, the license comment is preserved, but the documentation says the default is false.
https://swc.rs/docs/configuration/minification#jscminifyformat

@tbjgolden
Copy link
Author

esbuild has now re-enabled their comment preservation, so swc is the only minifier that I'm aware of that has this disabled by default

See for rationale:
evanw/esbuild#2745

@ishowta
Copy link

ishowta commented Dec 20, 2022

@tbjgolden I was just looking into minifier license preservation too, at least if I don't specify a config file, the license comments are preserved. I confirmed that Nextjs13 (which uses swc internally) also preserve the license comments. please see my above comment.

@kdy1 kdy1 added this to the Planned milestone Dec 21, 2022
@kdy1 kdy1 self-assigned this Dec 21, 2022
kdy1 added a commit that referenced this issue Dec 21, 2022
@kdy1 kdy1 modified the milestones: Planned, v1.3.25 Jan 5, 2023
@swc-bot
Copy link
Collaborator

swc-bot commented Feb 4, 2023

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@swc-project swc-project locked as resolved and limited conversation to collaborators Feb 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging a pull request may close this issue.

5 participants