w3ctag / design-principles

A small-but-growing set of design principles collected by the TAG while reviewing specifications
https://w3ctag.github.io/design-principles
177 stars 46 forks source link

Non-async methods that preform conversion should be prefixed with to* #134

Open kenchris opened 5 years ago

kenchris commented 5 years ago

According to @marcoscaceres non-async methods that preform conversion should be prefixed with to like toJSON(): https://github.com/w3c/web-nfc/pull/240#pullrequestreview-262273143

[NewObject] Promise<any> json();
[NewObject] Promise<USVString> text();
[NewObject] Promise<ArrayBuffer> arrayBuffer();

vs

[NewObject] toJSON();
[NewObject] USVString toText();
[NewObject] ArrayBuffer toArrayBuffer();
kenchris commented 5 years ago

Also relevant: https://heycam.github.io/webidl/#idl-tojson-operation

kenchris commented 5 years ago

@reillyeon @Honry

marcoscaceres commented 5 years ago

Note above that “any” should not be there.

domenic commented 5 years ago

I'm unsure whether this is uniformly followed; it'd be good to see more examples. In particular I'm skeptical that async vs. non-async methods should have different conventions.

marcoscaceres commented 5 years ago

heh, thanks for blaming me @kenchris! :P @domenic is right that it's all over the place, but we could try to find some "best practice" and bring some order going forward.

I guess it really depends what the API is doing... if it's converting "to" something, then uniform toWhatever() makes sense, irrespective of it being sync/async.

Fetch's Body mixin is a little different. It's requesting aspects of whatever Body is mixed into, and that can have side effects (like the body of a request being consumed or whatever).

annevk commented 5 years ago

Well, model-wise the body is part of the mixin.

convertToBlob() is another precedent we have here.

kenchris commented 5 years ago

@marcoscaceres the underlying data structure is bytes (potentially with a mime type) so it can be interpreted as text (UTF-8 decode), an array buffer, or parsed as JSON.

marcoscaceres commented 5 years ago

Quick grep in Gecko:

DOMMatrix.webidl:    [Throws] Float32Array      toFloat32Array();
DOMMatrix.webidl:    [Throws] Float64Array      toFloat64Array();
DOMMatrix.webidl:    [Throws] Float32Array      toFloat32Array();
DOMMatrix.webidl:    [Throws] Float64Array      toFloat64Array();
DOMMatrix.webidl:    [Default] object           toJSON();
DOMPoint.webidl:    [Default] object toJSON();
DOMQuad.webidl:    [Default] object toJSON();
DOMRect.webidl:    [Default] object toJSON();
HTMLCanvasElement.webidl:  DOMString toDataURL(optional DOMString type = "",
OffscreenCanvas.webidl:  Promise<Blob> toBlob(optional DOMString type = "",
PaymentAddress.webidl:  [Default] object toJSON();
PaymentResponse.webidl:  [Default] object toJSON();
Performance.webidl:  [Default] object toJSON();
PerformanceEntry.webidl:  [Default] object toJSON();
PerformanceNavigation.webidl:  [Default] object toJSON();
PerformanceNavigationTiming.webidl:  [Default] object toJSON();
PerformanceResourceTiming.webidl:  [Default] object toJSON();
PerformanceServerTiming.webidl:  [Default] object toJSON();
PerformanceTiming.webidl:  [Default] object toJSON();
PushSubscription.webidl:  PushSubscriptionJSON toJSON();
RTCIceCandidate.webidl:  [Default] object toJSON();
RTCSessionDescription.webidl:  [Default] object toJSON();
Selection.webidl:  DOMString  toStringWithFormat(DOMString formatType, unsigned long flags, long wrapColumn);
URL.webidl:  USVString toJSON();

convert*

dom/webidl/SVGAngle.webidl:  void convertToSpecifiedUnits(unsigned short unitType);
dom/webidl/SVGLength.webidl:  void convertToSpecifiedUnits(unsigned short unitType);
dom/webidl/TestInterfaceJS.webidl:  DOMString convertSVS(USVString svs);
kenchris commented 5 years ago

@marcoscaceres what about without the to*? grepping for blob(), json(), text() etc

Honry commented 5 years ago

Just have a quick grepping in https://github.com/web-platform-tests/wpt/tree/master/interfaces, where includes almost all the webidl files.

~/work/upstream/honry/web-platform-tests/interfaces$ grep "blob()" -r *
fetch.idl:  [NewObject] Promise<Blob> blob();
push-api.idl:  Blob blob();
~/work/upstream/honry/web-platform-tests/interfaces$ grep "json()" -r *
fetch.idl:  [NewObject] Promise<any> json();
push-api.idl:  any json();
~/work/upstream/honry/web-platform-tests/interfaces$ grep "text()" -r *
fetch.idl:  [NewObject] Promise<USVString> text();
FileAPI.idl:  [NewObject] Promise<USVString> text();
push-api.idl:  USVString text();
LeaVerou commented 3 years ago

@kenchris discussed this in a breakout today.

We see these patterns:

marcoscaceres commented 3 years ago

A prefix is not common in async functions so we should recommend against it

Yeah, on reflection and looking at the above, that makes sense.