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

Confirmation that the stability definition is correct #12701

Closed
sam-github opened this issue Apr 27, 2017 · 9 comments
Closed

Confirmation that the stability definition is correct #12701

sam-github opened this issue Apr 27, 2017 · 9 comments

Comments

@sam-github
Copy link
Contributor

cf. #12670 (comment)

@addaleax thinks the stability definition is wrong for experimental:

https://nodejs.org/api/documentation.html#documentation_stability_index

I don't agree, and would like confirmation that the docs are correct.

Node cannot break the ecosystem by changing or removing its APIs without going through its deprecation process (even the deprecation process can cause lots of kick-back, we all remember Buffer).

The definition of experimental explicitly states that the API is subject to change in a future version. It doesn't even say "future major version"! This means that experimental APIs are explicitly NOT subject to the deprecation process.

While any individual package author is (hopefully) aware that they are using an experimental API with no stability guarantees when they type their javascript code out that uses the API, and are comfortable with the risk, users of their package have no way to know. Node doesn't write warning messages to stderr when experimental APIs are called (perhaps it should?), npm doesn't refuse to install dependencies that rely on experimental APIs (how could it know?), so its very easy for a user to become dependent on experimental features.

In my opinion, the case for gating experimental APIs via a command line flag is to force the end consumer of a package, perhaps a consumer seperated by many layers of package depencies, to explicitly recognize that they are depending on experimental node APIs, and expect no guarantee of stability.

Doing this allows node to treat experimental APIs the way the docs say we do: experiments we can change or withdraw at will.

Without a flag, we cannot do that. experimental APIs are part of our API, and we can't and won't break them (whatever the docs say).

Inspector is an odd case of experimental. It was gated behind a flag, but only because all debugging is gated :-). It will still be behind a CLI flag when its not experimental.

URL is a case of something that should have been gated behind a CLI flag, or declated stable from the start. It was developed based on a fairly stable specification, and doesn't appear to have ever had breaking changes proposed (that I noticed), but if we had been forced to make breaking changes it could have been very painful for the ecosystem.

Those are the past, though, I'm more worried about the future, and would like to see agreement that the text is correct, and describes our intentions with any future experimental APIs.

@sam-github
Copy link
Contributor Author

@nodejs/ctc

@jasnell
Copy link
Member

jasnell commented Apr 27, 2017

Putting experimental things behind a flag is not something that we have consistently done. I'm +1 on @addaleax's interpretation on this and recommend that the description in the docs be updated to match actual practice.

@gibfahn
Copy link
Member

gibfahn commented Apr 27, 2017

I think that having $NODE_OPTIONS (#12028) makes putting things behind a flag much more feasible.

Putting experimental things behind a flag is not something that we have consistently done.

I get that we haven't done it in the past, why we wouldn't want to always do this in the future? Users depending on an immature API seems like a real problem, and not one that's fixable after the fact.

@jasnell
Copy link
Member

jasnell commented Apr 27, 2017

why we wouldn't want to always do this in the future

Because I don't see it as being necessary if use of the API is entirely opt in and the documentation is clear about it being experimental.

@targos
Copy link
Member

targos commented Apr 27, 2017

URL is a case of something that should have been gated behind a CLI flag, or declated stable from the start. It was developed based on a fairly stable specification, and doesn't appear to have ever had breaking changes proposed (that I noticed), but if we had been forced to make breaking changes it could have been very painful for the ecosystem.

I disagree with that. We had to make several changes to the URL implementation for spec compliance and they would have been semver-major if the stability index wasn't experimental.

@sam-github
Copy link
Contributor Author

@targos You quoted to much, what is "that"? You disagree it should have been behind a flag? You disagree it could have been painful for the ecosystem? You disagree that there were breaking changes that I noticed? Surely not that last.

@jasnell We had APIs in node for years (cluster, I'm looking at you) that node labelled experimental, but were treated wrt. to stability as "cannot change". There is a long history of not being able to change APIs even with experimental blazoned across the top, which frustrates the purpose of experimental.

This is not the case:

use of the API is entirely opt in and the documentation is clear about it being experimental.

The author of the code may know they are depending on experimental, but the indirect consumer of the API via npm dependencies has literally no way to know ATM if they depend on experimental APIs.

To be clear: I don't have a problem with experimental APIs, I don't have a problem with breaking them, I don't have a problem with URL or inspector or anything else we want to get community feedback on being published in node as an experimental API, I have a problem with not having experimental APIs be explicitly opt-in, making npmjs.org a minefield.

Also, I quite like the idea of experimental APIs emitting process warnings. That could be an alternative to CLI opt-in. Its not opt-in, but it allows the end user to know they have inherited a dependency on experimental code.

@jasnell
Copy link
Member

jasnell commented Apr 27, 2017

I'm certainly open to having future experimental APIs emit warnings when used.

@refack
Copy link
Contributor

refack commented Apr 27, 2017

[question]
Is it your experience that people always update the node executable?

[opinion]
From my experience there are two "general" cases:

  1. developers - usually update often, get frustrated when stuff breaks -> push-back. But they have the flexibility and knowledge how to adapt.
  2. production - once a system works everything is frozen (to the point of ignoring CVE fixing semver-patch updates), because they don't have the flexibility to adapt.

IMHO we need to think of these two separate groups' needs, and constraints/flexibility

[radical opinion]
If we feel we are doing something really important, we should be able to withstand push-back from frustrated devs, since they have the means to adapt.

jasnell added a commit to jasnell/node that referenced this issue May 11, 2017
* Update the experimental status to reflect actual common use.
* Also make a few formatting fixes since I'm in here.

Fixes: nodejs#12701
@Fishrock123
Copy link
Contributor

The definition of experimental explicitly states that the API is subject to change in a future version. It doesn't even say "future major version"! This means that experimental APIs are explicitly NOT subject to the deprecation process.

I would say this is correct. By default, they are not subject to it but we may consider it if so necessary for our users.

addaleax pushed a commit that referenced this issue Jul 24, 2017
* Update the experimental status to reflect actual common use.
* Also make a few formatting fixes.

Fixes: #12701

PR-URL: #12723
Fixes: #12701
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
MylesBorins pushed a commit that referenced this issue Aug 16, 2017
* Update the experimental status to reflect actual common use.
* Also make a few formatting fixes.

Fixes: #12701

PR-URL: #12723
Fixes: #12701
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
MylesBorins pushed a commit that referenced this issue Aug 16, 2017
* Update the experimental status to reflect actual common use.
* Also make a few formatting fixes.

Fixes: #12701

PR-URL: #12723
Fixes: #12701
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
MylesBorins pushed a commit that referenced this issue Sep 3, 2017
* Update the experimental status to reflect actual common use.
* Also make a few formatting fixes.

Fixes: #12701

PR-URL: #12723
Fixes: #12701
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
MylesBorins pushed a commit that referenced this issue Sep 5, 2017
* Update the experimental status to reflect actual common use.
* Also make a few formatting fixes.

Fixes: #12701

PR-URL: #12723
Fixes: #12701
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants