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.

Sign up for GitHub

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

Jump to bottom

Add alwaysInclude & filters options #1326

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add alwaysInclude & filters options #1326

wants to merge 2 commits into from

Conversation

gwatts
Copy link

@gwatts gwatts commented Jan 13, 2023

Resolves #1092

Adds a alwaysInclude option that takes an array of packages (or a function returning an array) to always include in the third party dependency set.

license({
  thirdParty: {
    output: path.join(__dirname, 'dist', 'dependencies.txt'),
    alwaysInclude: [ './node_modules/tailwindcss/package.json' ],
    }
  },
})

Adds a filter option that lets you add a function to filter out (or potentially add more, or merge sets together, which is the use case i was solving for) arbitrary dependencies.

license({
  thirdParty: {
    output: path.join(__dirname, 'dist', 'dependencies.txt'),
    filter: (deps) => deps.filter((dep) => !dep.name.include('mycorp'),
  },
})

@gwatts gwatts changed the title Add filters Add alwaysInclude & filters options Jan 13, 2023
@mjeanroy mjeanroy force-pushed the master branch 12 times, most recently from 0bcc216 to 5f4ed57 Compare March 11, 2024 08:30
@msidorov-flowbox
Copy link

Hello.
Any chance of getting this PR merged?
Thanks in advance.

@mjeanroy
Copy link
Owner

mjeanroy commented May 2, 2024

@msidorov-flowbox Hi,

I could definitely accept a PR that would implement some kind of filter or exclude option.

For the alwaysInclude, I'm not a big fan of this, as it opens other questions that are not so easy to answer (in my opinion):

  • What should the alwaysInclude look like?
    • If it's a package name, getting the README/License field from the package.json dependency is error prone, as the dependency path might change depending on the package manager and how the dependency is specified (is it a direct or transitive dependency? For transitive dependencies, is the dependency hoisted or not? Etc.)
    • If it is some kind of absolute path/relative path, what should be the base path?
  • What if the dependency cannot be found in the dependency tree, should it fail the build or not?

I'm open to discuss it, but I don't think it's a good idea to merge this part "as is".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add option to keep license for certain tree-shaken dependencies
3 participants
@gwatts @msidorov-flowbox @mjeanroy