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

Add fund subcommand #273

Closed
wants to merge 5 commits into from
Closed

Conversation

ruyadorno
Copy link
Collaborator

Overview

This PR introduces the npm fund command that lists all funding info provided by the installed dependencies of a given project.

✏️ Changes

  • lib/utils/funding.js Provides helpers to validate funding info and
    return a tree-shaped structure containing the funding data for all deps.
  • lib/fund.js Implements npm fund <pkg> command
  • Added tests
    • npm install mention of funding
    • npm fund <pkg> variations
    • unit tests for added lib/utils and lib/install helpers
  • Added docs for npm fund, funding package.json property
  • Fixed lib/utils/open-url to support --json config
  • Documented unicode on npm install docs

🔗 References

📸 Screenshots

Simple npm install within a project with given dependencies

Demo showing simple npm install within a project with given dependencies

npm install a package that contains dependencies with funding declared, fires up npm ls and npm fund to show differences in output

Demo showing npm install a package that contains dependencies with funding declared, fires up npm ls and npm fund to show differences

npm fund --json output example

Demo showing npm fund --json example

🔍 Testing

Install a package that has a valid funding object declared on its package.json, e.g:

  • Single package: npm install sleepover
  • Many nested packages with funding info: npm install @ruyadorno/test-funding-dep

✅ This change has unit test coverage
✅ This change has integration test coverage

🔥 Rollback

If we observe something wrong with this change in production, how should we respond?

This can be reverted at any time

kemitchell and others added 2 commits October 30, 2019 17:38
PR-URL: npm#246
Credit: @kemitchell
Close: npm#246
Reviewed-by: @ruyadorno

Thanks @kemitchell for providing the initial work that served as a base
for `npm fund`, its original commits messages are preserved as such:

- support: add support subcommand
- support: fix request caching
- support: further sanitize contributor data
- doc: Fix typo
- support: simplify to just collecting and showing URLs
- install: improve `npm support` test
- install: drop "the" before "projects you depend on"
- doc: Reword mention of `npm support` in `package.json` spec
This commit introduces the `npm fund` command that lists all `funding`
info provided by the installed dependencies of a given project.

Notes on implementation:

- `lib/utils/funding.js` Provides helpers to validate funding info and
return a tree-shaped structure containing the funding data for all deps.
- `lib/fund.js` Implements `npm fund <pkg>` command
- Added tests
  - `npm install` mention of funding
  - `npm fund <pkg>` variations
  - unit tests for added `lib/utils` and `lib/install` helpers
- Added docs for `npm fund`, `funding` `package.json` property
- Fixed `lib/utils/open-url` to support `--json` config
- Documented `unicode` on `npm install` docs

Refs: https://github.com/npm/rfcs/blob/2d2f00457ab19b3003eb6ac5ab3d250259fd5a81/accepted/0017-add-funding-support.md
@ruyadorno ruyadorno requested a review from a team as a code owner October 31, 2019 00:29
@brodybits
Copy link

I would personally favor some way to advertise which project maintainers are available for hire with some quick details or have any other services to offer (no third-party ads please).

For example (true example): "@brodybits is available for short-term hire in Boston, NYC, or remote contract"

@darcyclarke
Copy link
Contributor

@brodybits Hey Chris! Appreciate the feedback but I think it's out of the scope of this work which was talked about & simplified pretty extensively in the original RFC. If you have an idea about a future feature/design we could support, definitely feel free to contribute an RFC over on our npm/rfcs repo and/or, where I think your suggestion may make the most sense, is to investigate the work the Node Foundation Package Maintenance Working Group has done around a more verbose support schema.

@ruyadorno ruyadorno added this to In Review in Community & Open Source Oct 31, 2019
@ruyadorno ruyadorno added the semver:minor new backwards-compatible feature label Oct 31, 2019
@giuseppeg
Copy link

This looks amazing! It would be nice to have the info deduplicated and flattened and maybe have an advanced mode (maybe a --tree flag) to show how the packages are related.

There seem to be a lot of "noise" in the output from the first gif, a flat list with a white line to break every package info would be more readable.

@ljharb
Copy link
Collaborator

ljharb commented Nov 1, 2019

If i haven’t chosen to add a dep to package.json, it’s critical that i know how and why a dep is in my graph, or else why would i ever want to give it money?

@giuseppeg
Copy link

@ljharb fair but imagine having 50 packages with sub deps. The list could still be flat and each field can say "Used in: babel-core" or something (assuming babel-core is the dependency you installed).

@ljharb
Copy link
Collaborator

ljharb commented Nov 1, 2019

it could, but presumably you’d want the list sorted, with direct deps at the top, and then transitive deps sorted by “used in the most packages”. (iow, I’m partially agreeing with you; a tree isn’t the best way to represent this since duplicated deps should be consolidated and summarized)

@giuseppeg
Copy link

Lock files (yarn.lock and package-lock.json) are flat, I'd personally go for that. That said probably shipping any solution and asking the community for feedback is the best way to go.

@coreyfarrell
Copy link

A potential issue is that this is version based rather than package based? This means that I cannot set funding information for already released versions of a package. More importantly if funding information changes after a release I cannot fix it (remove a no longer valid funding method for example).

@ruyadorno
Copy link
Collaborator Author

@npm/cli-team: @giuseppeg raised an important point that went a bit under documented in my original description which is that the current implementation of npm fund output does some smart things when building that tree, such as moving transitive deps up in the tree if their parents has no funding info (since it only displays packages that contain such info) along with the stacking of same-maintainer info demoed in the 2nd gif there.

I quickly put together a demo showing how childs will show up a level above compared to a npm ls output:

in this demo the dependency@ruyadorno/dep-sub-no-funding@1.0.0 which has no funding info is removed from the npm fund listing and its child are listed as children of its parent

Demo showing diff between npm ls and npm fund when a parent dep has no funding info

the takeaway here is that this output is neither an exact representation of the dependency tree nor a flat list, it's something in between and my idea when working on it is that it's a middle-ground to start with and we can iterate later on with feedback from the community

@brodybits
Copy link

it’s critical that i know how and why a dep is in my graph

@ljharb yarn why would do that if you use Yarn.

I think this should be supported by npm, looks like there is already a npm-why package to do this.

@ljharb
Copy link
Collaborator

ljharb commented Nov 1, 2019

@brodybits i don't need to know that information in general, and i know how to get it without using yarn - i'm talking about directly inline in the funding info, which is where i'd need to see it to gain motivation to donate money to a transitive dep.

@ljharb
Copy link
Collaborator

ljharb commented Nov 1, 2019

@ruyadorno and what if test-funding-dep-sub-sub-sub is indirectly used by 23 different top-level packages? must i see it listed 23 times, or must its provenance only be shown for one ancestor and the other 22 lost?

@ruyadorno
Copy link
Collaborator Author

or must its provenance only be shown for one ancestor and the other 22 lost?

☝️ that is the case yes, in its current implementation

@ljharb
Copy link
Collaborator

ljharb commented Nov 1, 2019

That seems like proof that this is not the optimal output format to use.

// within top levels takes precedence over nested ones
return (Object.keys(deps)).map((key) => {
const dep = deps[key]
const { name, funding, version } = dep
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const { name, funding, version } = dep
const { name, version } = dep
const funding = dep.funding || dep.collective;

There are many packages using the collective property today to expose their Open Collective URL. Would be great to support that when funding is not found, while we wait for them to add it. I

The syntax of collective is exactly the same as funding, making the implementation uncomplicated.

It's also nice that this allow anyone to test the feature and see results today, while in the current state it's giving me only empty results for all the projects I tested.

@ruyadorno ruyadorno closed this in 4414b06 Nov 5, 2019
Community & Open Source automation moved this from In Review to Done Nov 5, 2019
ghost pushed a commit to rubygems/rubygems that referenced this pull request Dec 31, 2019
3060: Add `donation_uri` metadata field to gemspec r=bronzdoc a=colby-swandale

# Description:

I want to add a new metadata field to the gemspec called `donation_uri` for the purpose of linking a page which users can view on how to donate to/sponsor gem authors.

With the introduction of [Github Sponsors](https://github.com/sponsors), [Tidlelift](https://tidelift.com/), [Open Collective](https://opencollective.com) & [Patreon](https://www.patreon.com/), donating or sponsoring a developer is now super easy and streamlined. I hope by adding this field to accomplish a number of goals:

* Similar to npmjs.org, we add a button/ui element to the gem page on rubygems.org to link visitors to the given donation uri.
* Encourage tooling to be built to list information about how to donate to gem authors for a given Gemfile, Gemfile.lock. (similar to the new `npm fund` command npm/cli#273)
* Encourage more maintainers to signup and receive sponsorship for their work

# Tasks:

- [ ] Describe the problem / feature
- [ ] Write tests
- [ ] Write code to solve the problem
- [ ] Get code review from coworkers / friends

I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md).


Co-authored-by: Colby Swandale <me@colby.fyi>
ghost pushed a commit to rubygems/rubygems that referenced this pull request Dec 31, 2019
3060: Add `donation_uri` metadata field to gemspec r=bronzdoc a=colby-swandale

# Description:

I want to add a new metadata field to the gemspec called `donation_uri` for the purpose of linking a page which users can view on how to donate to/sponsor gem authors.

With the introduction of [Github Sponsors](https://github.com/sponsors), [Tidlelift](https://tidelift.com/), [Open Collective](https://opencollective.com) & [Patreon](https://www.patreon.com/), donating or sponsoring a developer is now super easy and streamlined. I hope by adding this field to accomplish a number of goals:

* Similar to npmjs.org, we add a button/ui element to the gem page on rubygems.org to link visitors to the given donation uri.
* Encourage tooling to be built to list information about how to donate to gem authors for a given Gemfile, Gemfile.lock. (similar to the new `npm fund` command npm/cli#273)
* Encourage more maintainers to signup and receive sponsorship for their work

# Tasks:

- [ ] Describe the problem / feature
- [ ] Write tests
- [ ] Write code to solve the problem
- [ ] Get code review from coworkers / friends

I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md).


Co-authored-by: Colby Swandale <me@colby.fyi>
ghost pushed a commit to rubygems/rubygems that referenced this pull request Jan 31, 2020
3060: Add `funding_uri ` metadata field to gemspec r=bronzdoc a=colby-swandale

# Description:

I want to add a new metadata field to the gemspec called `donation_uri` for the purpose of linking a page which users can view on how to donate to/sponsor gem authors.

With the introduction of [Github Sponsors](https://github.com/sponsors), [Tidlelift](https://tidelift.com/), [Open Collective](https://opencollective.com) & [Patreon](https://www.patreon.com/), donating or sponsoring a developer is now super easy and streamlined. I hope by adding this field to accomplish a number of goals:

* Similar to npmjs.org, we add a button/ui element to the gem page on rubygems.org to link visitors to the given donation uri.
* Encourage tooling to be built to list information about how to donate to gem authors for a given Gemfile, Gemfile.lock. (similar to the new `npm fund` command npm/cli#273)
* Encourage more maintainers to signup and receive sponsorship for their work

# Tasks:

- [ ] Describe the problem / feature
- [ ] Write tests
- [ ] Write code to solve the problem
- [ ] Get code review from coworkers / friends

I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md).


Co-authored-by: Colby Swandale <me@colby.fyi>
ghost pushed a commit to rubygems/rubygems that referenced this pull request Jan 31, 2020
3060: Add `funding_uri ` metadata field to gemspec r=bronzdoc a=colby-swandale

# Description:

I want to add a new metadata field to the gemspec called `funding_uri` for the purpose of linking a page which users can view on how to donate to/sponsor gem authors.

With the introduction of [Github Sponsors](https://github.com/sponsors), [Tidlelift](https://tidelift.com/), [Open Collective](https://opencollective.com) & [Patreon](https://www.patreon.com/), donating or sponsoring a developer is now super easy and streamlined. I hope by adding this field to accomplish a number of goals:

* Similar to npmjs.org, we add a button/ui element to the gem page on rubygems.org to link visitors to the given donation uri.
* Encourage tooling to be built to list information about how to donate to gem authors for a given Gemfile, Gemfile.lock. (similar to the new `npm fund` command npm/cli#273)
* Encourage more maintainers to signup and receive sponsorship for their work

# Tasks:

- [ ] Describe the problem / feature
- [ ] Write tests
- [ ] Write code to solve the problem
- [ ] Get code review from coworkers / friends

I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md).


Co-authored-by: Colby Swandale <me@colby.fyi>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:minor new backwards-compatible feature
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

8 participants