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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support file #220

Merged
merged 29 commits into from Aug 28, 2019

Conversation

@Eomm
Copy link
Member

Eomm commented Jun 14, 2019

Closes #218

Main changes:

  • the support tag become a support file
  • all the values are lower-case as suggested during the OpenJS Summit
  • add new keys as discussed in the issue

I think the funding field is not complete, any feedback is appreciated 馃憤

@ljharb

This comment has been minimized.

Copy link
Member

ljharb commented Jun 14, 2019

I'm still not on board with it being a separate file.

@dominykas

This comment has been minimized.

Copy link
Contributor

dominykas commented Jun 15, 2019

Personally I'm also in favor of keeping it in package.json, but can we make it work in both - package.json and support.json? Just need to define which one takes precedence when both are present?

@ljharb

This comment has been minimized.

Copy link
Member

ljharb commented Jun 15, 2019

If it's a local file path, sure - but not if it's a URL, since that adds a layer of indirection and mutability and ephemerality.

@ghinks

This comment has been minimized.

Copy link
Contributor

ghinks commented Jun 15, 2019

Apologies @ljharb could you give an example of what you meant, I am not sure I understood the issue.

@ljharb

This comment has been minimized.

Copy link
Member

ljharb commented Jun 15, 2019

If it鈥檚 a local file, or in package.json, then once I鈥檝e installed the package, without making any further network requests (important for, for example, enterprise CI installing from a private registry), i have all the support info for the package. If it鈥檚 a url, i need internet access to be able to validate and/or analyze it.

docs/drafts/PACKAGE-SUPPORT.md Outdated Show resolved Hide resolved
docs/drafts/PACKAGE-SUPPORT.md Outdated Show resolved Hide resolved
docs/drafts/PACKAGE-SUPPORT.md Outdated Show resolved Hide resolved
@mhdawson

This comment has been minimized.

Copy link
Member

mhdawson commented Jun 17, 2019

@ljharb I had the same initial reaction to having the data outside the package itself but I agree with the comment that support can change for a version without a new version being released and that it can be retroactive. ie sticking with version 1.1 which says the package is supported when version 1.2 says it is not in no way means it is still supported. Further, if I run tooling on the 1.1 package I'll actually get the wrong answer.

I'd much prefer that the data all be local as well after install but still struggling with how we account for this issue.

@ljharb

This comment has been minimized.

Copy link
Member

ljharb commented Jun 17, 2019

@mhdawson i think when support changes, a new release should be required - that way it flows into everyone's pre-existing update notification mechanisms.

@mhdawson

This comment has been minimized.

Copy link
Member

mhdawson commented Jun 17, 2019

@ljharb I agree a new release is good, however, that does not address the case where I run a tool that looks at what I've already installed and the local data would be wrong, particularly if versions of the dependencies are pinned.

Maybe we can do both. Local file, with an entry with a url for the latest version? This would let anybody who just wants to validate locally do that, while at the same time allow more sophisticated tools do an up to date check by following the URL?

@ljharb

This comment has been minimized.

Copy link
Member

ljharb commented Jun 17, 2019

I'd rather just have the contract for the support field be "latest chronological publish date trumps all previous ones", and keep all the data inline.

@mhdawson

This comment has been minimized.

Copy link
Member

mhdawson commented Jun 17, 2019

@ljharb just thinking about how a tool would work in that context.

It would check if the current version is the latest, and if not get the latest into some temporary directory and extract the file from there? Repeat for all dependencies? One concern might be the larger download size as you'd need to pull the full updated module.

@pi0

This comment has been minimized.

Copy link
Contributor

pi0 commented Jun 17, 2019

We can leverage from a service like unpkg to fetch the latest package's support.json for that range. (This is single-point-of-failure rather than random-point-of-failure if we allow any arbitrary URL)

@ljharb

This comment has been minimized.

Copy link
Member

ljharb commented Jun 17, 2019

@mhdawson npm show package@version fieldname will include the support info if it's inline, without ever having to download the package - making the registry the only point of failure. (test this out with, eg, npm show enzyme@2 license or whatever)

@wesleytodd

This comment has been minimized.

Copy link
Member

wesleytodd commented Jun 17, 2019

i think when support changes, a new release should be required

This is not practical in reality. When an author drops off the map they are unlikely to publish a update to say unsupported. I am not saying that the external support definition will fix this, but it seems more likely that an author would share a single support method for many packages, so updating a single place is more likely than having to publish every package.

IMO it is a small price to pay to have the tooling make more requests to make the information more likely to be accurate. Also it would reduce the work for maintainers, which is, after all, one of the main goals of this WG :)

@ljharb

This comment has been minimized.

Copy link
Member

ljharb commented Jun 17, 2019

@wesleytodd i think the likelihood of an author dropping off the map with no effort made - ie, not running npm deprecate, not changing support declarations anywhere, etc - is pretty high.

@wesleytodd

This comment has been minimized.

Copy link
Member

wesleytodd commented Jun 17, 2019

Do you think there is a reasonable "default" time frame where it is expected that we could expire a support declaration if no update is made? Like if we said that anything older than 2 years is marked as expired?

@pi0

This comment has been minimized.

Copy link
Contributor

pi0 commented Jun 17, 2019

@wesleytodd indeed expires field is for this purpose. We may also consider a default for it's value.

@wesleytodd

This comment has been minimized.

Copy link
Member

wesleytodd commented Jun 17, 2019

@pi0 That is what I meant, setting a default. The con of this approach would be that in case there are no changes but support is unchanged would require a "bump" if the default expiration data was hit. IMO this would be preferable over having millions of unsupported packages with out of date support.json's.

@vweevers

This comment has been minimized.

Copy link

vweevers commented Jun 17, 2019

Also it would reduce the work for maintainers, which is, after all, one of the main goals of this WG

馃挴 Off-topic: I feel like this group has been catering to users too much, which doesn't help the panic (see fx koa-router), inflated vulnerability reports (see execa, axios) and increased false sense of entitlement that I've been noticing in the JS ecosystem ever since the event-stream incident. I had hoped that this incident - among others - would lead to education of users (e.g. how to interpret package vulnerability reports in the context of an application) and tooling for users (like #77), rather than more work for maintainers.

@wesleytodd

This comment has been minimized.

Copy link
Member

wesleytodd commented Jun 17, 2019

@vweevers While I agree with the problem statement, I think that we are focusing in the right areas. Helping the users helps the maintainers. The single most important, stressful and time consuming thing maintainers face, in my experience, is helping users聽through these kind of issues.

As long as the solution we create does not cause more work for the maintainers, helping the users is helping the maintainers.

@ljharb

This comment has been minimized.

Copy link
Member

ljharb commented Jun 17, 2019

@wesleytodd i think a default expires makes lots of sense, and requiring a bump to refresh it seems fine to me too.

@mhdawson

This comment has been minimized.

Copy link
Member

mhdawson commented Jun 18, 2019

@ljharb thanks for pointing me to npm show. That should work although it does require that the information be in the package.json as opposed to an external file right?

@ljharb

This comment has been minimized.

Copy link
Member

ljharb commented Jun 18, 2019

@mhdawson yes, that鈥檚 correct.

@arcanis

This comment has been minimized.

Copy link

arcanis commented Aug 25, 2019

I find this proposal interesting, I just have one meta question: is it expected this will have any effect on package managers (such as surfacing this information somehow)? If so, shouldn't the relevant communities have been pinged along the way?

For the JavaScript ecosystem it is recommended that this information be added to the existing
`package.json` file in the root directory of the project. For other ecosystems, it is recommended that
this information be stored in a file called `package-support.json` in the root directory of the
project.

This comment has been minimized.

Copy link
@voxpelli

voxpelli Aug 25, 2019

This seems to expect that the project is a downloadable package?

What if the project is instead provided over eg. HTTP? Then a /.well-known/<something> would be preferable?

Also: Has it been considered for such a file to also be discoverable through a link relation similar to rel-license links? Like a <link rel="backing" href="<url-to-a-backing-json-somewhere>.json" /> on the home page of the project?

Or is it intended that all discovery of this data should just happen through a published package and the tools that interact with this package and that there is no use for eg. a browser extension to surface this very same information by making it discoverable on the web?

This comment has been minimized.

Copy link
@ljharb

ljharb Aug 25, 2019

Member

The latter paragraph. The target audience for the information is the developers consuming the package, not anybody interacting with it via a website.

This comment has been minimized.

Copy link
@wesleytodd

wesleytodd Aug 25, 2019

Member

The scope here is purposefully limited to packages (with a strong focus on node packages in the npm registry). That said, I do not see any reason not to take the json format here and publish it as ./well-known/support.json or any other version you mention. I don't think we should add that to the spec at this time, but it seems a fine use of the shared schema.

@ljharb

This comment has been minimized.

Copy link
Member

ljharb commented Aug 25, 2019

@arcanis npm鈥檚 been involved throughout; it鈥檇 be great if you wanted to join the package maintenance working group.

Do you have any thoughts about the proposal?

@arcanis

This comment has been minimized.

Copy link

arcanis commented Aug 25, 2019

Sounds good! I don't have much bandwidth to track all discussions on too many places, so if you think something might be of interest to me, a ping would go a long way 馃槉

Do you have any thoughts about the proposal?

A few. I've opened similar issues in the past (opencollective/opencollective#1625, yarnpkg/yarn#6971), so I'm interested to solve that as well.

My main thoughts:

  • The format of the package.json field doesn't matter much imo - what matters is what package managers will do with it by default. I think this spec should be designed top-down, not bottom-up. What information do we want to surface to the users, and how?

  • The format is quite verbose, and I wonder whether it might be because it tries to do too much from the very beginning.

    • For example I wouldn't put response-paid in the initial spec, as I'm not sure it'll be used by a lot of packages - even by those who would have a "paid plan", as they might have multiple levels of sponsorship.
  • This proposal is very "fixed in time". I guess the expires field is one way to work around this problem, but why not just keep the support information outside of the package itself? For example, why not define "support" as an url that points to an external location that would follow the same spec?

    • Does it really need to be statically computable apart from "does this package defines some level or support or not" (which checking for the mere presence of the support field would already allow)? Do we even need to be able to statically compute this property (cf next section)?

    • One extra problem with these data being immutable: should I republish a release if I want to change the support level? If I make a mistake when publishing? If I want to take holidays? If I want to start offering support when there was none?

  • This is definitely an open-source problem, but is it really an information that can / should be set in the packages themselves? Could the right fix come from the registries (for example GitHub sponsors) rather than the packages themselves? I'd like to be sure this spec considers why this is the best layer to implement this feature.

    • To better get my point, consider how packages are deprecated on the npm registry: you run a command that tells the registry that the whole family of packages is now deprecated. You don't set a "deprecated" field in your package.json and publish it.
@ljharb

This comment has been minimized.

Copy link
Member

ljharb commented Aug 25, 2019

The only immutable thing that can be relied on is a package registry - not GitHub or a url - so the best place for it to live is inside the package tarball. Yes, you should publish a release if you want to change the support level; whatever鈥檚 under the 鈥渓atest鈥 tag obsoletes all the others. When the registry supports updating specific files (like a package-support.json, for example) without a publish (also fetching the info without downloading the entire tarball), then that鈥檇 be the mechanism we recommend instead.

@ljharb

This comment has been minimized.

Copy link
Member

ljharb commented Aug 25, 2019

It鈥檚 a fair point that if the registry directly supported hosting support info, that鈥檇 be a superior approach, but that seems like something that should happen after this approach gains adoption.

@arcanis

This comment has been minimized.

Copy link

arcanis commented Aug 25, 2019

The only immutable thing that can be relied on is a package registry - not GitHub or a url - so the best place for it to live is inside the package tarball.

What I mean is that support information are not immutable, so storing them inside immutable structures might be counter-productive.

For example, if the field was an url, users would be able to set it to a live document that would explain the support constraints as they currently are. So my question is: what makes immutability a feature rather than a drawback?

@ljharb

This comment has been minimized.

Copy link
Member

ljharb commented Aug 25, 2019

But the support information needs to be immutably accessed. URLs disappear even if support does not, and it鈥檚 valuable to know when support ended, forever.

@arcanis

This comment has been minimized.

Copy link

arcanis commented Aug 25, 2019

I'm not sure I understand why? Especially in the case of support information - if they become unavailable for any reason (for example a 404), then it seems reasonable to interpret it as "all support for this package has been stopped".

(Edit: I guess 5xx errors are one type where the interpretation above doesn't necessarily work, but it seems fair to just treat it as "undecided" and suggest to retry again at a later time - those information aren't required for the package to be functional)

@wesleytodd

This comment has been minimized.

Copy link
Member

wesleytodd commented Aug 25, 2019

Hey @arcanis, welcome! It might be a better use of everyone's time if you read through this thread. We have covered most of these points either here or on the WG calls, so I will not respond to those again right now. You can find those videos linked in the meeting notes: https://github.com/nodejs/package-maintenance/tree/master/meetings

The format is quite verbose, and I wonder whether it might be because it tries to do too much from the very beginning.

I rather disagree. We identified only 4 dimensions of support, and the spec has just enough guidance so that we can cover most use cases we know about without being a free-for-all. If you can recommend ways we can cover the same information in a "less verbose" way I would be happy to change the format, but I think the information covered is the right info.

even by those who would have a "paid plan", as they might have multiple levels of sponsorship.

Multiple support levels is covered by the proposal.

This proposal is very "fixed in time".

The proposal is very much not fixed in time. It even has a section about updating via publish and how the latest tag is the definitive version. This is all about evolving support models.

I'd like to be sure this spec considers why this is the best layer to implement this feature.

I think this is a reasonable critique. Do you have alternate proposal we could consider?

@arcanis

This comment has been minimized.

Copy link

arcanis commented Aug 25, 2019

It might be a better use of everyone's time if you read through this thread. We have covered most of these points either here or on the WG calls

I did read this thread and its document, but I simply cannot dig into multiple ones to put together the information and make the right interpretation of the words of many people (this PR references an issue which references Slack which references the OpenJS summit, plus the WG calls you mention). This is why I think it's even more important for an RFC to describe the rejected designs than the accepted one.

To anyone watching, I found the reasoning for this field not being an url here:

To me that's the exact reason it shouldn't be a file - support for a version is something you should commit to for the forseeable future, not something you should be able to revoke at any time. this, bu Whether it's part of the package tarball or not, it would need to have some sort of irrevocable/immutable way to communicate and update this information (like npm publishing), and neither a git repo, nor a URL, are those things.

I'm still not sure I entirely understand. This choice is explained by arguing that "a version is [...] not something you should be able to revoke at any time", but that goes into contradiction with "whatever鈥檚 under the 鈥渓atest鈥 tag obsoletes all the others" and "the latest tag is the definitive version". Is it the former or the later?

I think this is a reasonable critique. Do you have alternate proposal we could consider?

Imo the main question here, more than the support, is where to store the metadata associated to a project, not a version. Basically, we have a one-to-many relationship (one project has many versions). Where should that be stored? There are multiple options:

  • The project metadata can be copied into each version, and the "latest" wins. This is what this proposal does, if I understand correctly?

  • The project metadata can be referenced through a remote url, users would have to explicitly create this metadata set and reference it from their package. This is what I mentioned. One problem is that some versions might not point to those metadata.

  • The project metadata can be stored on the registry. The implementation would be similar to the second option, except that the registry would enforce that the metadata url is one pointing to its registry.

Note that the definition of "project" is loose if we have an url. For example, if we want to provide different support strategies for different branches (such as Node and its LTS), we simply have to provide a different metadata url to each version. For example:

"support": "https://registry.example.org/node/support/10"

This wouldn't be possible with the current proposal, which is a "all or nothing".

Multiple support levels is covered by the proposal.

How would you express that if you paid $5 you get response-7 and if you paid $500 you get response-1? I didn't see it covered by the RFC, and that's why I said that I'm not sure the RFC should even detail the response times apart from the OSS expected one and a boolean to indicate whether better responses are expected through a paid plan.

@voxpelli

This comment has been minimized.

Copy link

voxpelli commented Aug 25, 2019

The project metadata can be referenced through a remote url, users would have to explicitly create this metadata set and reference it from their package.

Another advantage of this is that multiple modules can share the same support information without having to keep possibly hundreds of modules up to date (which can happen when one eg. changes employment). For someone like @mafintosh that could be quite some work and I myself would not be that keen to duplicate how I maintain my modules across all of my modules.

@ljharb

This comment has been minimized.

Copy link
Member

ljharb commented Aug 25, 2019

It wouldn鈥檛 be difficult to build greenkeeper-like tooling to update such support info.

@Eomm

This comment has been minimized.

Copy link
Member Author

Eomm commented Aug 25, 2019

Hi @arcanis
I think the URL feature will be covered in future: #241

To anyone watching, I found the reasoning for this field not being an url here:

The reasons that I collected are:

  • the npm registry is planning, but it is not ready, to provide new features like:
    • show the support information in the npm site
    • add some warning like "this package is not supported" based on the user configuration
    • "publish" some files without changing the tarball - ex: update the README
  • we want to build tooling around this spec and the source of truth is the registry, not the repo since it can be deleted. So the information must be collected in the registry itself
    • because of this, if we put the info in a separate file, we should download the tarball instead of the package.json's metadata
  • the user can run npm show <module> support

So if you go out with a URL for the first alpha-test we could lose some important features that npm can implement and it would be a great loss.
The URL will be a relative path to the repo maybe, and point to an editable file in the npm registry.. right now is hard to say but I'm sure that npm is a key actor to spread this support information.
About this, you could watch the first part of this meeting since it is @ahmadnassri and he says some important thing!

The project metadata can be copied into each version, and the "latest" wins. This is what this proposal does, if I understand correctly?

Yes, it is

How would you express that if you paid $5 you get response-7 and if you paid $500 you get response-1?

Right now you can't set prices, so in this case, the data should be:

response: [
  {
    "type": "regular-7",
    "paid": true,
    "contact": "Not so fast support <john-slow@foo.com>"
  },
  {
    "type": "regular-1",
    "paid": true,
    "contact": "Fast support <john@foo.com>"
  }
]

And it is up to the user decide which repose type ask for.

Hi @voxpelli too

Another advantage of this is that multiple modules can share the same support information without having to keep possibly hundreds of modules up to date

This is a good point and I agree with that totally. The solution we had in mind is to (finish and) publish a tool that can update the package.json of many repo opening a PR

@wesleytodd

This comment has been minimized.

Copy link
Member

wesleytodd commented Aug 25, 2019

IMO this is in a good state and should be merged. I know we just got a resurgence of comments, but I think we could probably split the conversation off into separate threads to discuss the details.

@wesleytodd wesleytodd referenced this pull request Aug 25, 2019
5 of 11 tasks complete
@voxpelli

This comment has been minimized.

Copy link

voxpelli commented Aug 26, 2019

@Eomm I have made such a tool, to the despair of colleagues whenever I update the CI setup across 100 repos and their notifications and our internal commit channel goes bananas. One thing not solved by such an approach is the semantic that those all use the same config. A consumer would have to treat, update and present them as 100 individual configs rather than as a single config covering 100 modules. From a UI perspective I would much rather be able to see that I use these 5 modules from Mafintosh and they have the shared support setup this and that, rather than getting that support setup iterated over 5 times. (Also, without the support policy explicitly mentioning the 5 modules sharing the same support policy, it鈥檚 hard to dedupe later in the chain, as that could easily give people the impression that they do share the same support policy)

@ljharb

This comment has been minimized.

Copy link
Member

ljharb commented Aug 26, 2019

@voxpelli you're assuming, though, that the common use case will be to support every module the same. While clearly some will do that, I don't think that's going to be universal.

I maintain almost 200 packages. Some of them I'd mark as "best effort", some as "regular-1", some as "regular-7", etc. A design that prioritizes making them all the same - thus penalizing making them different - isn't equitable for the wider ecosystem.

@voxpelli

This comment has been minimized.

Copy link

voxpelli commented Aug 26, 2019

@ljharb I鈥檓 not assuming that it will be the common use case, just saying that it is a use case and a use case which I myself am leaning towards and which I can see others lean towards.

I would probably like you make some exceptions from my baseline, but the majority of my projects would share the same setup and those with exceptions could maybe also in the end share a common setup, although one specifying a higher level of support.

Edit: I do am +1 on @wesleytodd in getting this merged here and now and then move on to (probably multiple) follow up threads

@mhdawson

This comment has been minimized.

Copy link
Member

mhdawson commented Aug 26, 2019

I'm also +1 on landing this, it is still in the "drafts" folder. The next step will be to publicize more to get broader input before moving it to what we recommend. In that context I think it makes sense to land it and use as a basis for the next set of discussions.

@mhdawson

This comment has been minimized.

Copy link
Member

mhdawson commented Aug 27, 2019

Discussed in package-maintenance meeting. Plan is to:

  • Land as is, it is draft still anyways
  • Write up blog which introduces it along with some of the contentious points (live/not live)
  • Use feedback after blog post to revise
  • Then promote to 'non-draft'
@darcyclarke

This comment has been minimized.

Copy link
Member

darcyclarke commented Aug 27, 2019

@mhdawson are we pulling this in? I'd like to comment on/revise the proposal but don't want to do that here (rather modify the merged, draft).

| `commercial` | The package is maintained and supported by a corporate entity as part of supporting their products.
| `paid-support` | The package is maintained and supported through paid support contracts.
| `freemium` | Basic version of the package it provided for free, premium version is available at a cost.
| `donations` | The project can be funded by any donations.

This comment has been minimized.

Copy link
@lastmjs

lastmjs Aug 28, 2019

I think we need to extend the donations field to allow for identifiers that would allow direct donations to the project. I'm coming at this from the context of my project Sustainus: https://github.com/lastmjs/sustainus

Essentially what I'm telling people to do right now is to put a field called ethereum at the root level of their package.json. This field's value is an Ethereum name or address, which allows funds to be sent directly to that address. Any application can parse the Ethereum information and send funds as it pleases, so it is not specific to Sustainus.

I think this would be useful for a number of cryptocurrencies, most notably Bitcoin and Ethereum, but we should prepare for others. This could also work for other payment systems like PayPal, or who knows even bank accounts (not likely).

Perhaps it could look like this:

{
  "donations": {
    "ethereum": "0xc9bc72fe58c5FAcc478514313FbA09d13ee3742b",
    "bitcoin": "bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq",
    "libra": "0x0a0d88E64da0CFB51d8D1D5a9A3604647eB3D131",
    "paypal": "coolproject@gmail.com"
  }
}

This comment has been minimized.

Copy link
@wesleytodd

wesleytodd Aug 28, 2019

Member

As I said on twitter, interesting use case and I think it should be supported in this spec. In this thread we decided to merge and then handle specific issues like this in follow up PRs. Would you be willing to wait until that happens and then open up a suggestion PR?

This comment has been minimized.

Copy link
@lastmjs

lastmjs Aug 28, 2019

Of course, I guess just let me know when and what to do and I'll do it

@mhdawson

This comment has been minimized.

Copy link
Member

mhdawson commented Aug 28, 2019

@darcyclarke just about to land. You can then open a PR with suggestions/changes.

@mhdawson mhdawson merged commit 5dd50b3 into nodejs:master Aug 28, 2019
@mhdawson

This comment has been minimized.

Copy link
Member

mhdawson commented Aug 28, 2019

@darcyclarke @lastmjs @wesleytodd Landed, please open follow on PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.