vercel / nft

Node.js dependency tracing utility
https://npmjs.com/@vercel/nft
MIT License
1.31k stars 136 forks source link

feat(resolve): export `resolve()` function #354

Closed gnoff closed 1 year ago

gnoff commented 1 year ago

nft allows you to override the resolve step with a hook however it does not expose its internal resolveDependency implementation. This means if you want to take over resolving in certain cases you must do so in every case. To make conditional or augmented resolving possible this change reexports the resolveDependency function from the index. This way you can use it in custom resolve hook implementations

gnoff commented 1 year ago

Lets also document it in the README since this is the default resolve() hook behavior.

Hmm this API leaves a little to be desired. there are 2 arguments to resolveDependency that aren't really public, the job and isCjs arguments.

Perhaps we ought to provide a resolve function to the resolve hook which curries these arguments to avoid exposing them. It's somewhat unfortunate that the extra arguments are already provided when they are not documented because we can't really change them now. I'll think a bit more about this

gnoff commented 1 year ago

I renamed the export and described it as it is currently implemented in the readme but still don't love the API

gnoff commented 1 year ago

@styfle is there something up with ci?

styfle commented 1 year ago

@gnoff Yep, fixed in https://github.com/vercel/nft/pull/355

styfle commented 1 year ago

we can't really change them now

We can change make breaking changes if we ship under a new version.

We haven't even hit 1.0.0 so breaking changes are expected https://github.com/vercel/nft/releases

That being said, I think we should ship what we have to unblock the work in Next.js and we can make changes if needed in a new PR in the future.

github-actions[bot] commented 1 year ago

:tada: This PR is included in version 0.23.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

gnoff commented 1 year ago

Yeah sounds good!