Browse Source

[js] make Promise interact properly with Thenable (closes #7655, closes #6028)

 - Promise unifies with Thenable
 - Promise.resolve return type is correct when Thenable/Promise is passed
 - make Thenable.then signature the same as Promise.then
 - rename callback arguments to conform with the names in ES spec

NOTE: this uses some hacks and workarounds related to #5785 and #7656 (see comments) - we need to revisit them when those issues are addressed
Dan Korostelev 6 years ago
parent
commit
fa2526c7bb

+ 11 - 8
std/js/Promise.hx

@@ -42,9 +42,8 @@ extern class Promise<T>
 		use `Promise.resolve(value)` instead and work with the return value as
 		a promise.
 	**/
-	@:overload(function<T>(promise : Promise<T>) : Promise<T> {})
-	@:overload(function<T>(thenable : Thenable<T>) : Promise<T> {})
-	static function resolve<T>( ?value : T ) : Promise<T>;
+	@:overload(function<T>(?value : T) : Promise<T> {})
+	static function resolve<T>( thenable : Thenable<T> ) : Promise<T>;
 
 	/**
 		Returns a Promise object that is rejected with the given reason.
@@ -79,7 +78,7 @@ extern class Promise<T>
 		its original settled value if the promise was not handled
 		(i.e. if the relevant handler onFulfilled or onRejected is not a function).
 	**/
-	function then<TOut>( fulfillCallback : Null<PromiseHandler<T, TOut>>, ?rejectCallback : PromiseHandler<Dynamic, TOut> ) : Promise<TOut>;
+	function then<TOut>( onFulfilled : Null<PromiseHandler<T, TOut>>, ?onRejected : PromiseHandler<Dynamic, TOut> ) : Promise<TOut>;
 
 	/**
 		Appends a rejection handler callback to the promise, and returns a new
@@ -87,7 +86,7 @@ extern class Promise<T>
 		or to its original fulfillment value if the promise is instead fulfilled.
 	**/
 	@:native("catch")
-	function catchError<TOut>( rejectCallback : PromiseHandler<Dynamic, TOut> ) : Promise<TOut>;
+	function catchError<TOut>( onRejected : PromiseHandler<Dynamic, TOut> ) : Promise<TOut>;
 }
 
 /**
@@ -95,12 +94,16 @@ extern class Promise<T>
 **/
 abstract PromiseHandler<T,TOut>(T->Dynamic) // T->Dynamic, so the compiler always knows the type of the argument and can infer it for then/catch callbacks
 	from T->TOut // order is important, because Promise<TOut> return must have priority 
-	from T->Promise<TOut> // although the checking order seems to be reversed at the moment, see https://github.com/HaxeFoundation/haxe/issues/7656
+	from T->Thenable<TOut> // although the checking order seems to be reversed at the moment, see https://github.com/HaxeFoundation/haxe/issues/7656
+	from T->Promise<TOut> // support Promise explicitly as it doesn't work transitively through Thenable at the moment
 {}
 
 /**
 	A value with a `then` method.
 **/
-typedef Thenable<T> = {
-	function then(resolve:T->Void, ?reject:Dynamic->Void):Void;
+@:forward
+abstract Thenable<T>(ThenableStruct<T>) from ThenableStruct<T> {} // abstract wrapping prevents compiler hanging, see https://github.com/HaxeFoundation/haxe/issues/5785
+
+typedef ThenableStruct<T> = {
+	function then<TOut>( onFulfilled : Null<PromiseHandler<T, TOut>>, ?onRejected : PromiseHandler<Dynamic, TOut> ) : Thenable<TOut>;
 }

+ 1 - 1
tests/misc/projects/Issue6790/compile-fail.hxml.stderr

@@ -1,2 +1,2 @@
 Mismatch.hx:6: characters 19-26 : (e : Unknown<0>) -> String should be Null<js.PromiseHandler<Dynamic, Int>>
-Mismatch.hx:6: characters 19-26 : For optional function argument 'rejectCallback'
+Mismatch.hx:6: characters 19-26 : For optional function argument 'onRejected'

+ 28 - 0
tests/misc/projects/Issue7655/Main.hx

@@ -0,0 +1,28 @@
+import js.Promise;
+
+class Main {
+	static function main() {
+		var p:Thenable<String> = new Promise<String>(null);
+
+		$type(p.then(function(x) $type(x)));
+		$type(p.then(function(x) $type(x), function(e) $type(e)));
+
+		$type(p.then(function(x) {$type(x); return 1;}));
+		$type(p.then(function(x) {$type(x); return Promise.resolve(1);}));
+
+		$type(p.then(null,function(x) {$type(x); return 1;}));
+		$type(p.then(null,function(x) {$type(x); return Promise.resolve(1);}));
+
+		$type(p.then(function(x) {$type(x); return 1;}, function(e) {$type(e); return 1;}));
+		$type(p.then(function(x) {$type(x); return Promise.resolve(1);}, function(e) {$type(e); return 1;}));
+		$type(p.then(function(x) {$type(x); return 1;}, function(e) {$type(e); return Promise.resolve(1);}));
+		$type(p.then(function(x) {$type(x); return Promise.resolve(1);}, function(e) {$type(e); return Promise.resolve(1);}));
+
+		$type(Promise.resolve(p));
+		$type(Promise.resolve(new Promise<String>(null)));
+		$type(Promise.resolve("hi"));
+
+		// should not hang, see https://github.com/HaxeFoundation/haxe/issues/5785
+		$type(p.then(_ -> p));
+	}
+}

+ 8 - 0
tests/misc/projects/Issue7655/Mismatch.hx

@@ -0,0 +1,8 @@
+import js.Promise;
+
+class Mismatch {
+	static function main() {
+		var p:Thenable<String> = new Promise<String>(null);
+		p.then(x -> 10, e -> "");
+	}
+}

+ 3 - 0
tests/misc/projects/Issue7655/compile-fail.hxml

@@ -0,0 +1,3 @@
+-main Mismatch
+-js whatever.js
+--no-output

+ 2 - 0
tests/misc/projects/Issue7655/compile-fail.hxml.stderr

@@ -0,0 +1,2 @@
+Mismatch.hx:6: characters 19-26 : (e : Unknown<0>) -> String should be js.PromiseHandler<Dynamic, Int>
+Mismatch.hx:6: characters 19-26 : For optional function argument 'onRejected'

+ 3 - 0
tests/misc/projects/Issue7655/compile.hxml

@@ -0,0 +1,3 @@
+-main Main
+-js whatever.js
+--no-output

+ 29 - 0
tests/misc/projects/Issue7655/compile.hxml.stderr

@@ -0,0 +1,29 @@
+Main.hx:7: characters 34-35 : Warning : String
+Main.hx:7: characters 9-37 : Warning : js.Thenable<Void>
+Main.hx:8: characters 34-35 : Warning : String
+Main.hx:8: characters 56-57 : Warning : Unknown<0>
+Main.hx:8: characters 9-59 : Warning : js.Thenable<Void>
+Main.hx:10: characters 35-36 : Warning : String
+Main.hx:10: characters 9-50 : Warning : js.Thenable<Int>
+Main.hx:11: characters 35-36 : Warning : String
+Main.hx:11: characters 9-67 : Warning : js.Thenable<Int>
+Main.hx:13: characters 40-41 : Warning : Unknown<0>
+Main.hx:13: characters 9-55 : Warning : js.Thenable<Int>
+Main.hx:14: characters 40-41 : Warning : Unknown<0>
+Main.hx:14: characters 9-72 : Warning : js.Thenable<Int>
+Main.hx:16: characters 35-36 : Warning : String
+Main.hx:16: characters 70-71 : Warning : Unknown<0>
+Main.hx:16: characters 9-85 : Warning : js.Thenable<Int>
+Main.hx:17: characters 35-36 : Warning : String
+Main.hx:17: characters 87-88 : Warning : Unknown<0>
+Main.hx:17: characters 9-102 : Warning : js.Thenable<Int>
+Main.hx:18: characters 35-36 : Warning : String
+Main.hx:18: characters 70-71 : Warning : Unknown<0>
+Main.hx:18: characters 9-102 : Warning : js.Thenable<Int>
+Main.hx:19: characters 35-36 : Warning : String
+Main.hx:19: characters 87-88 : Warning : Unknown<0>
+Main.hx:19: characters 9-119 : Warning : js.Thenable<Int>
+Main.hx:21: characters 9-27 : Warning : js.Promise<String>
+Main.hx:22: characters 9-51 : Warning : js.Promise<String>
+Main.hx:23: characters 9-30 : Warning : js.Promise<String>
+Main.hx:26: characters 9-23 : Warning : js.Thenable<String>