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

WHATWG URL API is marked as Experimental, but is not behind a flag #11362

Closed
ChALkeR opened this issue Feb 14, 2017 · 15 comments
Closed

WHATWG URL API is marked as Experimental, but is not behind a flag #11362

ChALkeR opened this issue Feb 14, 2017 · 15 comments
Labels
doc Issues and PRs related to the documentations. whatwg-url Issues and PRs related to the WHATWG URL implementation.

Comments

@ChALkeR
Copy link
Member

ChALkeR commented Feb 14, 2017

Previous discussion in #11200 (comment), but this deserves a separate issue.

Per the documentation:

Stability: 1 - Experimental
This feature is subject to change, and is gated by a command line flag.
It may change or be removed in future versions.

That explicitly states that all Stability: 1 - Experimental API in Node.js should be hidden behind a command line flag, but The WHATWG URL API (introduced in #7448) is marked as Experimental but is available without a command-line flag in v7.x.

This means that people might miss the «Experimental» label (especially given that it's description is contradictory) and url.URL could be used in the wild, which limits how we could change things there (making url.URL not so experimental-y in fact).

Also note that the current Accepting Modifications/Breaking Changes/Deprecations policy doesn't differentiate between Experimental and Stable APIs, it only cares about the API being public (which url.URL certainly is), documented (which it also is), or used in the wild (which it probably also is by now).

/cc @nodejs/ctc and especially @jasnell, also @sam-github and @gibfahn

@ChALkeR ChALkeR added doc Issues and PRs related to the documentations. whatwg-url Issues and PRs related to the WHATWG URL implementation. labels Feb 14, 2017
@ChALkeR
Copy link
Member Author

ChALkeR commented Feb 14, 2017

Personal opinion on this:

I'm not even completely sure if we should keep the whole «Stability index» thing around. Locked is mostly broken (see #6528), Experimental also seems misused in some situations (see #11200 (comment) by @sam-github). And we can't call url.URL explicitly Stable because there could be upstream changes that are out of our direct control if we want to be compatible with those. Also the policies don't care about Experimental or Locked if the API is public.

I think that with recent clarifications in the COLLABORATOR_GUIDE.md and the deprecation policy (#7964) we need only Experimental label for labelling things that are behind a flag, Deprecated label for labelling things that are deprecated per the deprecation process, and everything else should be covered by the identical regular rules from the COLLABORATOR_GUIDE.md, without calling them The API has proven satisfactory or any further division. That means treating all public API as Stable, as we mostly do now and as it's documented in the Collaborator Guide.

To clarify: I'm not proposing to change the process here, but this would in fact bring the documentation into line with the actual process.

@gibfahn
Copy link
Member

gibfahn commented Feb 14, 2017

I think that with recent clarifications in the COLLABORATOR_GUIDE.md and the deprecation policy (#7964) we need only Experimental label for labelling things that are behind a flag, Deprecated label for labelling things that are deprecated per the deprecation process, and everything else should be covered by the identical regular rules from the COLLABORATOR_GUIDE.md,

So Experimental, Deprecated, and the default (basically Stable)? SGTM


For this specific issue though, I guess if it's semver-major to add a flag then URL will have to be de facto stable for the life of v7, which is only till June (LTS schedule chart). I assume we won't put URL in Node 8, so it could be behind a flag for Node v9 in October.

@jasnell
Copy link
Member

jasnell commented Feb 14, 2017

I would just update the stability to index description to say may be gated behind a flag. For URL, there's simply no reason to put it behind a flag

@gibfahn
Copy link
Member

gibfahn commented Feb 14, 2017

@jasnell I think the argument for putting it behind a flag was made by @sam-github in #11200 (comment)

Except that historically, introducing an API means it cannot be changed without breaking code, and people scream when their code breaks, the fact that having said we reserve that right doesn't make them feel better.

The putting it behind a flag means that it is actually impossible for a node module to depend on this feature unless the ultimate consumer of that module specifically passes a flag to node when they invoke it, which is a fairly strong indicator of willingness to live on the edge. It means both the author of the js code depending on the experimental feature, and the consumer of the js code explicitly indicate their willingness to use experimental code. Otherwise, its easy for someone to accidentally use an API and not notice its experimental, and its almost impossible for a consumer of a module to know that happened.

In the absence of the passing of a command line flag, I don't think we should be breaking our APIs - everything should be treated as 'stable', and we should go through a deprecation/warning cycle if we want to introduce a breaking change.

@joyeecheung
Copy link
Member

I think the concept of "breaking" is a little bit different when it comes to WHATWG URL, because the API doesn't come from Node.js. If we apply the stability index to it as-is, it might never fit the description of a "Stable" API, even if we polish it enough for broader usage. Suppose it becomes "Stable", and the spec introduces a breaking change or we catch up the spec compliance with a breaking fix (though at this point those should probably be just edge-cases), if we can't backport it to LTS or a Stable release because it is "breaking" from Node.js's perspective, then users are stuck with a incompatible implementation and this makes the idea of a Web standard implemented in Node less attractive.

@TimothyGu
Copy link
Member

TimothyGu commented Feb 14, 2017

if we can't backport it to LTS or a Stable release because it is "breaking" from Node.js's perspective, then users are stuck with a incompatible implementation and this makes the idea of a Web standard implemented in Node less attractive.

IMO, stable is stable, and all promises we have made to our users by the instrument of Semantic Versioning cannot be broken just because the API originally came from another source.

On the other hand, I trust the authors of WHATWG URL Standard to recognize the fact that an incompatible change in the standard is not to be taken lightly, and that when such a change is made, not only would Node.js become incompatible, so will the Internet browsers installed on billions of computers worldwide.

@TimothyGu
Copy link
Member

Regarding the issue of flagging, I personally don't think it is necessary. require('url').URL doesn't really affect any pre-existing APIs, so the issue of backward compatibility is minimal; for those who do start using it, chances are they have read the documentation (and see the large "experimental" sign) because it is a new feature in v7. Of course, input from the community would be ideal.

I would also not go so far as to say that the idea of a stability index is to be abolished completely, since I believe what we have right now is clear and easy to understand (especially when compared to the version used in pre-io.js Node.js), and certainly useful. However, I shall defer to the opinion of @nodejs/ctc members.

@joyeecheung
Copy link
Member

@TimothyGu By incompatible I mean incompatible with the spec, not necessarily incompatible with general usage. Browsers still do breaking changes if they only break edge cases, but Node.js might have different opinions about what is an "edge case", especially in the scope of an LTS release. This is probably just the limitation of semver though, because a lot of bugfixes and API additions are just breaking in nature, even though not intended and only breaks edge cases, like adding record<USVString, USVString> to URLSearchParams constructor.

Although at this point I would say changes coming out of the spec and already implemented by browsers should not be semver-major, because if they don't break the Web, they don't break Node.

@TimothyGu
Copy link
Member

TimothyGu commented Feb 14, 2017

then users are stuck with a incompatible implementation and this makes the idea of a Web standard implemented in Node less attractive.

By incompatible I mean incompatible with the spec, not necessarily incompatible with general usage.

Indeed. But the fact is that even the most current of browsers are incompatible with the current spec. If Web developers can deal with older versions of the spec, so can Node.js developers.

Although at this point I would say changes coming out of the spec and already implemented by browsers should not be semver-major because if they don't break the Web, they don't break Node.

The definition of "implemented" is quite vague. Implemented in Chrome Canary? or Stable? or Firefox Nightly or Stable? If we follow Chrome Canary and bleeding-edge spec, we don't really have much guarantee that the change doesn't "break the Web"; and if we align ourselves with Chrome Stable, with the current Chrome release cadence it'll take ~2.5 months for Canary to reach Stable anyway. For Firefox, this period is even longer (4-5 months, if the change isn't fast-tracked to a more stable release series).

@joyeecheung
Copy link
Member

I mean stable releases of browsers, because the time they need from experimental builds(or something alike) to stable releases is what "don't break the Web" entails.

@joyeecheung
Copy link
Member

joyeecheung commented Feb 14, 2017

^ and I was referring to the backport process (where semver labels matter), not the implementation process.

@joyeecheung
Copy link
Member

joyeecheung commented Feb 14, 2017

Indeed. But the fact is that even the most current of browsers are incompatible with the current spec. If Web developers can deal with older versions of the spec, so can Node.js developers.

I might be making a bad analogy here, but not backporting spec fixes to a LTS makes it an IE6 of Node.js releases :P It's not about whether it is compatible now, but about whether it will receive fixes and how frequent that would be. IE7 and later don't help the situation of IE6 if that's what users are stuck with.

@ChALkeR
Copy link
Member Author

ChALkeR commented Feb 14, 2017

@jasnell

I would just update the stability to index description to say may be gated behind a flag. For URL, there's simply no reason to put it behind a flag

Key questions:

  • Is the backwards compatibility of that API is expected to be broken in a non semver-major version?
  • Is the backwards compatibility of that API is expected to be broken in a semver-major version without the usual documented deprecation process?
  • The same two questions about the potential removal of the Experimenal API.
  • The same four questions about the API that is actually behind a flag (e.g. lib: remove simd support from util.format() #11346).

The documentation should be clear about what changes should the users expect and per what process, from «might be removed in a semver-patch» to «requires several semver-major versions per the deprecation process» — that clarification of the expetations is exactly what those stability indexes were originally for, but they don't serve that purpose now. (Note: Node.js didn't even follow semver back then when those were introduced).

@jasnell
Copy link
Member

jasnell commented Feb 14, 2017

I would not expect frequent significant breaking changes as that's not the approach that the WHATWG takes to evolving it's specs. Changes are likely to be fairly small for the most part. We are, however, just getting going with this and will need to work out a process that makes sense. We do not control the specification but we do have input on it (they've been reaching out to us for input on a number of items and the relationship has been very productive so far). Bottom line is that it's not 100% clear what the process will ultimately need to be but I do not anticipate much variance from our existing semver policy

@ChALkeR
Copy link
Member Author

ChALkeR commented May 29, 2017

Fixed in 10754b6.

@ChALkeR ChALkeR closed this as completed May 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. whatwg-url Issues and PRs related to the WHATWG URL implementation.
Projects
None yet
Development

No branches or pull requests

5 participants