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
feat: remove dependency on arborist, require tree to be passed in #128
Conversation
BREAKING CHANGE: the arborist tree must now be provided in the options and will not be generated for you. the npm-packlist bin has also been removed.
5d61936
to
d20b5da
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with a tiny nit on the dev dep. But this looks ready to ship.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do i get a nice error message if i fail to provide the tree?
This also uses `tree.path` as the fallback instead of `process.cwd()`. Arborist defaults to `cwd` as well so there is no functional difference. This also refactors away some dead code now that tree is required. BREAKING CHANGE: `tree` is now the first parameter
} | ||
|
||
super(options) | ||
this.isPackage = options.isPackage | ||
this.seen = options.seen || new Set() | ||
this.tree = options.tree // no default, we'll load the tree later if we need to | ||
this.tree = tree |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this should have some kind of runtime check, so that if something not-tree-like is passed, it eagerly throws an exception.
BREAKING CHANGE: the arborist tree must now be provided in the options and will not be generated for you. the npm-packlist bin has also been removed.