-
Notifications
You must be signed in to change notification settings - Fork 13
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
filterPackages
callback option
#63
Conversation
I haven't added a test yet as I thought I'd make sure the API approach is ok. For those wishing to see an example, this is how I'm using it ( filterPackages (unflattenedPackages) {
const packages = [];
const flatten = (pkg) => {
packages.push(pkg);
if (pkg.children) {
pkg.children.forEach((p) => flatten(p));
}
};
unflattenedPackages.forEach((p) => flatten(p));
// console.log('packages', packages);
const filteredPackages = packages.filter((pkg) => {
// Ensure we are getting a package with the version set in the
// user's package.json
// Could also be a normal dep. if, e.g., copying for browser;
// but normally will be whitelisting devDep. that we are copying
// over
// const isRootDep = pkg.package._requiredBy.includes('#USER');
const isRootDevDep = pkg.package._requiredBy.includes('#DEV:/');
return isRootDevDep && bundledRootPackages.includes(pkg.name);
});
// eslint-disable-next-line jsdoc/require-jsdoc
function getDeps (pkgs) {
pkgs.forEach((pkg) => {
if (!pkg) {
return;
}
const {package: {dependencies}} = pkg;
if (dependencies) {
const pkgsToCheck = [];
Object.keys(dependencies).forEach((dep) => {
const findPkg = (pk) => {
if (!pk) {
return false;
}
const {name} = pk;
return dep === name;
};
/* eslint-disable unicorn/no-fn-reference-in-iterator */
if (filteredPackages.find(findPkg) !== undefined) {
return;
}
const pk = packages.find(findPkg);
/* eslint-enable unicorn/no-fn-reference-in-iterator */
pkgsToCheck.push(pk);
filteredPackages.push(pk);
});
getDeps(pkgsToCheck);
}
});
}
getDeps(filteredPackages);
return filteredPackages;
} |
@kemitchell : Hi--was just wondering if I could ask--did you have reservations about adding this, or do you just need some time to review? |
There was discussion in #53, but I thought I remembered comments from others, too. If you need this functionality now, by all means run a fork. @brettz/licensee on the public registry isn't going to irk me or anyone else. I'm very grateful for this PR, especially seeing how politely and respectfully you sent it. But my personal priority for this package, to the extent I have time, is all the change dependencies are going to force on us pretty soon. See #64. |
Thank you for the status update, @kemitchell . As far as @ljharb 's comments on legal liability, that doesn't apply in my case. I am not seeking to ignore transitive dependencies (on the contrary, for the packages being bundled, I want to include all transitive dependencies), but instead am checking a subset of devDependencies that are being bundled. In a number of repos, I use npm for versioning, but have a copy script to copy In such repos, if I were to only look at |
@brettz9 the license terms of some devDependencies may affect what you want to do, even if you don't bundle or redistribute their code. For example, projects under RPL, Parity, or nonstandard, custom terms. |
Sure, and that is good to know, but one can have a separate process for checking that. I want to indicate what is being bundled. One can already opt to only check |
It sounds like you don't want a filter so much as to control all the input to a "do these things meet the license rule" algorithm. |
Yes, you could put it that way. I think I am technically filtering to only those devDeps of interest, but yes, that is what I'm seeking to do (and why the rest of your code is still helpful to me as it does that). |
The filter in this PR is filtering those at root which end up getting checked, not filtering out transitive deps along the way. |
… which packages have their children processed
As this just adds a single boolean check for existing users, I'm wondering whether you would be ok to merge this? It'd be great not to have to publish a separate fork just for this if this use case could be accommodated. As mentioned, I've been using https://github.com/brettz9/license-badger in various projects based on this, and has been working rather well. I'm happy to rename the config to |
v8.1.0 |
filterPackages
callback to filter which packages have their children processedThis fixes #53 .
Rather than adding the complexity of directly supporting filtering for specific packages and their children, this adds a simple callback option by which the user can do so. It is an intermediate solution for @kemitchell 's helpful suggestion. It avoids the user having to run
readFilesystemTree
separately on their own.(In case anyone was wondering why I did not express as
packages = packages.filter(configuration.filterPackages)
, this per-item filtering would not be sufficient, as the filtering needed for determining dependencies is not linear; that is why the callback receives and returns a wholepackages
array:packages = configuration.filterPackages(packages)
.)