promises-aplus / promises-spec

An open standard for sound, interoperable JavaScript promises—by implementers, for implementers.
https://promisesaplus.com/
Creative Commons Zero v1.0 Universal
1.84k stars 164 forks source link

change "if x is a promise" to something non-circular and clear #240

Open donhatch opened 7 years ago

donhatch commented 7 years ago

The promises/A+ spec currently has a clause saying "If x is a promise, adopt its state [3.4]:" where footnote [3.4] is: "Generally, it will only be known that x is a true promise if it comes from the current implementation. This clause allows the use of implementation-specific means to adopt the state of known-conformant promises."

Notice that, according to the earlier definition of "promise", "If x is a promise" means "If x is an object or function with a 'then' method whose behavior conforms to this specification", where "this specification" includes the clause in question.

In other words, the definition of "promise" is circular.

If [3.4] is an attempt to clarify, it fails.

The clause in question should be changed to something non-circular and clear:

In particular, it should be possible to discern whether each of the following possible implementations of the boolean function SOMETHING(x) is conformant:

donhatch commented 7 years ago

I think one reasonable possible resolution would be to change "If x is a promise, adopt its state [3.4]:" to "(Optional) If x is known to be a promise, adopt its state [3.4]:".

Even though this is still self-referential, I believe this will make it clear and meaningful.

In particular, the questions of whether the 6 SOMETHING(x) functions shown at the end of my original post are conformant can then be answered: NO, NO, YES (assuming we trust the two implementations are conformant to the entire spec), YES, NO, NO.

ForbesLindesay commented 7 years ago

x instanceof Promise is the kind of check that 3.4 is suggesting. "whose behavior conforms to this specification" is the key part of the "promise" definition here. The only way to know than x conforms to this specification is if you verify that it is a known promise implementation. If you know which promise implementation is being used, it's ok to use a more optimised approach to adopt the state of that promise.

If you feel this can be made more clear, perhaps you can submit a pull request?

donhatch commented 7 years ago

Thanks @ForbesLindesay . From what you're saying, it looks like my interpretation of this clause is correct.

I'll put together a pull request with the wording I suggested in my previous comment, maybe also adding some of your words to footnote 3.4.

donhatch commented 7 years ago

I made the pull request (#241) and revised it a few times based on review discussion. I'm now very happy with it; I think it completely resolves the problem.

For reference, the proposed new wording is:

(Recommended) If `x` is known to be a promise, adopt its state [3.4]:
...
[3.4] Generally, it will only be known that `x` is a true promise if it comes from the
current implementation.  This clause allows the use of implementation-specific means to
adopt the state of known-conformant promises, which may be identified by a test such as
`x instanceof Promise`. This allows optimisations by not requiring the more general
thenable-handling procedure with its repeated value inspection.

It looks to me like @ForbesLindesay, @bergus, and @briancavalier are on board with the above proposed new wording. But @domenic isn't; in his Nov 1 comment:

In general I am -1 on this change. I don't think the current spec is ambiguous and people haven't had any issues implementing it many times.

So we're back to the question of whether to make any change here at all. I'll try to make one more appeal for it here. Here goes.


@domenic, I get that you think this part of the spec is good enough and not worth putting much more thought into.

The others (@ForbesLindesay, @bergus, @briancavalier) have put thought into it, and they've arrived at this tighter version of this section that I think is really good and completely resolves the issue.

Unless you actually think this change makes it worse overall, would you be willing to let the change through?

Here's why I think it's important.

The world is learning how promises work, in large part through discussions that refer to the promises/A+ spec, since it's the primary and best reference. In fact, it's the only reference that has the level of precision and completeness that's appropriate and useful for most of these discussions. (In contrast, the ES6 promises spec, for example, in addition to being implementation-specific, uses language that is too complicated and convoluted for it to be useful in resolving most of the questions I've seen.)

In particular, the wording of this section becomes significant when I discuss your spec with people, e.g. when asking or answering questions on stackoverflow, or if I write an implementation, or when I review someone else's implementation. Without the proposed tightening, I find myself saying things of the form:

According to the promises/A+ spec: [old wording], which obviously can't be taken literally since that wouldn't make sense, but I've talked to the spec authors about it and I can tell you with confidence that they did have something coherent in mind here, and what they really meant was: [new wording].

which is kind of wordy and embarrassing.

Acknowledged, people have managed to implement it many times in spite of this hurdle, and maybe all of them even inferred your intent correctly-- I haven't checked.

In any case, after this change, I'll be able to say the following instead, which I very much prefer:

According to the promises/A+ spec: [new wording].

That's a significant reduction in noise and distraction which will increase the quality of the global discussion, and it will also reflect better on you and on the great work you've put into this project.

What do you think? Will you let it through?

donhatch commented 4 years ago

@ForbesLindesay, what is the status of this?

My reading of what has happened so far is

Does this depend on domenic? @ForbesLindesay, would you be able to accept the pull request?

domenic commented 4 years ago

I am paying attention and my objection stands.

donhatch commented 4 years ago

Hi Domenic,

Great that you're paying attention.

Please give an argument supporting your "I don't think the spec is ambiguous" that addresses the issue and the replies I've given you so far. Your offhand dismissal without explanation is unhelpful and comes off as disrespectful of the thought and work that I and the others have put into describing and attempting to address the issue.

Your "people haven't had any issues implementing it many times." is specious, and (even if reworded without the hyperbole), I suspect, false. I ask you to either start engaging thoughtfully and respectfully, or get out of the way.