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

Suggestion: Add ability to paste LICENSE.* files into third-party dependencies file #379

Closed
lukastaegert opened this issue Jun 8, 2019 · 8 comments

Comments

@lukastaegert
Copy link
Contributor

When searching for a way to bundle files for distribution on npm in a license-compatible manner and nearly starting to write my own plugin I came across yours which is really close to what I have in mind but would need some extensions to solve the issue at hand.

At the moment, your plugin is able to generate a file listing the licenses of third-party dependencies. While this is really helpful for other people using my package to get an overview of applying licenses, it does not meet the requirements of many of these licenses. For instance MIT states that "... The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. ..." while ISC states "... provided that the above copyright notice and this permission notice appear in all copies. ..."

Obviously, these requirements are not met by simply listing the licenses. One idea to adress this without a lot of complicated logic and keeping the "single file" approach could be to scan for "LICENSE.*" files next to package.json files and add an option to paste their content into the third party dependencies file. That would meet the requirements of many FOSS licenses and would also make me recommend this plugin to everyone bundling their dependencies before publishing.

If you would be open to a PR please let me know, but there are some more ideas for which I will open separate issues.

@mjeanroy
Copy link
Owner

Hi @lukastaegert,

Thanks for the report. What you suggest looks pretty nice, I'll implement it (if you want to submit a PR, that's ok, but I think you already have a lot of work with rollup!).

Do you have a suggestion about the option name? Something like includeCopyright or includeLicense? Also, I suggest to add a warning if there is no LICENSE file available for the given dependency, what do you think?

@lukastaegert
Copy link
Contributor Author

I think includeLicense should be easier for the user to understand as the file is called ´LICENSEas well. If we assume everyone follows Github's recommendation https://help.github.com/en/articles/adding-a-license-to-a-repository#including-an-open-source-license-in-your-repository we should be ok with only checking forLICENSEandLICENSE.md`.

Another thing I keep wondering about (but this could also be a separate feature suggestion) is to allow the user more formatting control over the third party licenses file. An easy way to do that could be to allow thirdParty to be a function and if that is the case, pass it an array with information about all dependencies (i.e. all information included in the auto-generated file, including a license key). With that, the user could:

  • Control formatting if they want
  • Generate a header containing e.g. information about all licenses found (would be helpful for consumers of the package)
  • Add their own license to the top to generate a single license file for everything
  • Implement their own warning mechanism for problematic licenses (thus making Suggestion: Add option to warn about problematic or missing licenses #381 superfluous)

The previous object form could still be supported for a pre-formatted file as before. What do you think?

@mjeanroy
Copy link
Owner

Yeah, checking for the LICENSE or LICENSE.md is pretty easy, I can add it.

For the second part, I'm very sorry, I'm not sure to understand clearly your suggestion, may you just add a code example ? :)

@lukastaegert
Copy link
Contributor Author

Ah sorry, here is how it could work from a user perspective:

// this should still work
license({
  thirdParty: {
    output: path.join(__dirname, 'dist', 'dependencies.txt')
  }
})

// but this could be supported as well
license({
  thirdParty(dependencies) {
    fs.writeFileSync(path.join(__dirname, 'LICENSE', `This package contains code from the following packages:
${dependencies.map(({name, license, licenseFile}) => `${name}: ${license}\n${licenseFile}\n`)}`)
  }
})

On the plugin side, this could be like

if (typeof thirdParty === 'function') thirdParty(dependencies) else {/*old logic*/}

@mjeanroy
Copy link
Owner

Hi @lukastaegert,
I made several changes that should be a response to your issue.

First of all, the dependency object now include licenseText property which is the content of the LICENSE (can be LICENSE.md, LICENSE.txt, etc.) file found in the same directory as the package.json file of the dependency (may be null without any license file).
This license text will be now exported in the default output.

Also, now, as you suggested, the thirdParty option can be a function, such as:

license({
  thirdParty(dependencies) {
    // Do whatever you want.
  }
});

Here, only non private dependencies will be available as the first parameter.
If you want to include private dependencies, you can use the thirdParty.output option as a function too:

license({
  thirdParty: {
    includePrivate: true,
    output(dependencies) {
      // Do whatever you want.
    },
  },
});

Otherwise, you can still use the "good old" file output with a default content:

license({
  thirdParty: {
    includePrivate: true,
    output: 'path/to/thirdParty/output/file.txt,
  },
});

Finally, if you still want to use the output file, but customize the output text, you can use the thirdParty.output.template hook:

license({
  thirdParty: {
    includePrivate: true,
    output: {
      file: 'path/to/thirdParty/output/file.txt',
      template(dependencies) {
        return `This package contains code from the following packages: ${dependencies.map(({name, license, licenseFile}) => `${name}: ${license}\n${licenseText}\n`)}`;
      },
    },
  },
});

It allows you, for example, to export a JSON file instead of a text file:

license({
  thirdParty: {
    includePrivate: true,
    output: {
      file: 'path/to/thirdParty/output/file.json',
      template(dependencies) {
        return JSON.stringify(dependencies);
      },
    },
  },
});

Do you think it can solve your issue, or do you see something that should be added before releasing a new version (#381 will be fixed in the next version) ?

@lukastaegert
Copy link
Contributor Author

Awesome! I'll give it a spin!

@lukastaegert
Copy link
Contributor Author

This is amazing, great work!

@mjeanroy
Copy link
Owner

Version 0.12.0 has been published with these changes.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants