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

Support top-level await #471

Closed
ry opened this issue Aug 7, 2018 · 37 comments · Fixed by #3024
Closed

Support top-level await #471

ry opened this issue Aug 7, 2018 · 37 comments · Fixed by #3024

Comments

@ry
Copy link
Member

ry commented Aug 7, 2018

microsoft/TypeScript#25988

https://github.com/tc39/proposal-top-level-await

https://bugs.chromium.org/p/v8/issues/detail?id=9344

@ry ry added this to the future milestone Aug 7, 2018
@ry ry modified the milestones: future, v0.2 Aug 16, 2018
@ry ry added the enhancement label Aug 17, 2018
@ry ry modified the milestones: v0.2, future Aug 23, 2018
@kitsonk
Copy link
Contributor

kitsonk commented Sep 27, 2018

If we wanted this to work while we are waiting on TC39/TypeScript, we would have to do a bit of hackery.

When emitting a module with TLA, we would have to specifically ignore the diagnostic error related to that. TypeScript always emits, it is us who then chooses to throw. I might be a good idea to actually log a warning, since when TLA arrives, it could be a different behaviour to what we would have.

Then we would have to modify the AMD factory function to be async and deal with awaiting the promise resolution returned from that factory before we consider the module "resolved".

@kitsonk
Copy link
Contributor

kitsonk commented Sep 27, 2018

The drawback, like the module specifiers ending in .ts, we won't be able to get any of the editors to accept this code... so while it will run properly under deno, it won't look pretty in editors.

@ry
Copy link
Member Author

ry commented Sep 27, 2018

https://github.com/tc39/proposal-top-level-await#optional-constraint-top-level-await-can-only-be-used-in-modules-without-exports
Potentially it will be easier to do with this restriction. We only really need it for "main" scripts.

@ry ry removed the enhancement label Oct 5, 2018
@ry ry modified the milestones: future, v0.3 Nov 16, 2018
@ry ry modified the milestones: v0.3, future Jan 4, 2019
@aharonNF
Copy link

TC39 start promoting it.
Do we want to be part of it?
tc39/proposal-top-level-await#42

@kitsonk
Copy link
Contributor

kitsonk commented Feb 10, 2019

Well, one member of TC39 is trying to get it moving again. I am not sure we can really add anything to it. The biggest challenge is that they need to address the functional challenge that top-level await and how modules are evaluated and as Ry mentioned above, the optional restriction would make it a lot easier, but this is really needs the implementors to figure out how to do it, versus adding another voice to the mix.

@kitsonk
Copy link
Contributor

kitsonk commented Jun 6, 2019

Top level await has gone Stage 3. That means that the TypeScript team should be willing to implement it now.

@ry
Copy link
Member Author

ry commented Jul 17, 2019

A V8 issue has been created for this https://bugs.chromium.org/p/v8/issues/detail?id=9344

@kitsonk
Copy link
Contributor

kitsonk commented Aug 4, 2019

The TypeScript team are indicating that it will be challenging to downemit and are considering not allowing a downemit. That is find from our perspective, but that "problem" continues to delay them addressing the issue.

@ry
Copy link
Member Author

ry commented Sep 11, 2019

https://chromium.googlesource.com/v8/v8.git/+/591d1c9de4b3f2e4159c01d57f2c1e6e37d7cb41

TLA support has landed in V8

@kitsonk
Copy link
Contributor

kitsonk commented Sep 11, 2019

TLA is scheduled for TS 3.7 as well (early-November): microsoft/TypeScript#33352

@ry
Copy link
Member Author

ry commented Sep 12, 2019

Reverted v8/v8@93a29bd

@ry
Copy link
Member Author

ry commented Sep 16, 2019

Relanded v8/v8@798cb90

@ry ry mentioned this issue Sep 16, 2019
@ry
Copy link
Member Author

ry commented Sep 24, 2019

Parsing support has landed https://chromium.googlesource.com/v8/v8.git/+/0ceee9ad28c21bc4971fb237cf87eb742fc787b8

@ry
Copy link
Member Author

ry commented Sep 25, 2019

Deno v0.19 has integrated the changes from V8 and has a test demonstrating TLA.

However this is JS only support. We still need TS to not barf on TLA syntax. Therefore I'm leaving this issue open until we have a corresponding tests/top_level_await.ts.

@kitsonk
Copy link
Contributor

kitsonk commented Sep 25, 2019

That should be doable without ignoring error codes with TS 3.7 on Nov 5th.

@ry ry closed this as completed in #3024 Sep 30, 2019
@somombo
Copy link

somombo commented Oct 2, 2019

@kitsonk In this latest TS 3.7 announcement, there is no indication that there will be any support for TLA by Nov 5th.

@kitsonk
Copy link
Contributor

kitsonk commented Oct 2, 2019

That is the beta, and the announcement is what is in the beta. The iteration plan mentioned above still lists it. I am sure Ry and I will chat to the team about it next week at TSConf.

@hayd
Copy link
Contributor

hayd commented Oct 24, 2019

In case people are following this, here's v8 issues for top-level for-await (not merged yet):

https://bugs.chromium.org/p/v8/issues/detail?id=9817
https://bugs.chromium.org/p/v8/issues/detail?id=9825

@kitsonk
Copy link
Contributor

kitsonk commented Oct 24, 2019

Also, I had a chat with Daniel R, PM on TypeScript team, and TLA won't make TypeScript 3.7. It simply causes us to ignore the error message in Deno and will show it as invalid in editors, though it will run fine under Deno.

@somombo
Copy link

somombo commented Oct 24, 2019

In this latest TS 3.7 announcement, there is no indication that there will be any support for TLA by Nov 5th.

guess I was right then..
did Daniel R give a reason why it won't be included?

@kitsonk
Copy link
Contributor

kitsonk commented Oct 24, 2019

Yes, not enough time in the team. I think some of the other features took longer than they anticipated.

@hayd
Copy link
Contributor

hayd commented Oct 25, 2019

Actually, re-reading those commits above I think top-level for-await is in recent V8 versions. IIRC this means we can support it in ts easily once we upgrade V8.

@JavaScriptDude
Copy link
Contributor

I see that TypeScript now has first release with Top Level Await at 3.8. Does this help the progress of TLA in Deno?

@kitsonk
Copy link
Contributor

kitsonk commented May 8, 2020

Deno has TLA. You are commenting on a closed issue.

@JavaScriptDude
Copy link
Contributor

Thanks.

@anuragvohraec
Copy link

anuragvohraec commented May 11, 2020

Deno has TLA. You are commenting on a closed issue.

This example seems to not work :

function add(a: any,b: any,c: any): any{
	if(a+b>10){
		return c("My error",a+b);
	}else{
		return c(null, a+b);
	}
}

try{

 let t = await (async()=>{
	return add(4,5,(e: any, r: any)=>{
		if(e) throw e;
		return r;	
	});
 })();
 console.log(t);
}catch(e){
console.log("Yes! caught error from call back: ", e);
}

Gives me this error:
error TS2304: Cannot find name 'await'.

@kitsonk
Copy link
Contributor

kitsonk commented May 11, 2020

@anuragvohraec this is a TypeScript problem, and the error is a mis-leading... because you don't have any imports or exports in the module, TypeScript is treating this like a script instead of a module. This is causing the parsing in the try block to be a bit off, and instead of telling you that the module isn't a module error, it is giving you this strange error.

To work around the issue just add the following at the start or the end of the module:

export {};

@anuragvohraec
Copy link

To work around the issue just add the following at the start or the end of the module:

export {};

still same error :

export {};

function add(a: any,b: any,c: any): any{
	if(a+b>10){
		return c("My error",a+b);
	}else{
		return c(null, a+b);
	}
}

try{

 let t = await (async()=>{
	return add(4,5,(e: any, r: any)=>{
		if(e) throw e;
		return r;	
	});
 })();
 console.log(t);
}catch(e){
console.log("Yes! caught error from call back: ", e);
}

@JavaScriptDude
Copy link
Contributor

JavaScriptDude commented May 11, 2020

@anuragvohraec - Pardon my N00b question, but when would you use the (async(){...})() in real world logic in Deno?

From my understanding, all functions in Deno are implicitly async and I'm not sure if this makes this async()... methodology redundant?

@lucacasonato
Copy link
Member

No, not all functions in are implicitly async. Only the global scope (so the top level of a module outside of a function scope) and all function scopes declared with the async keyword.

await foo() // valid here

function test1() {
  await foo() // invalid here
}

async function test2() {
  await foo() // valid here
}

More info on top level await: https://v8.dev/features/top-level-await

@JavaScriptDude
Copy link
Contributor

@lucacasonato - Thanks, that makes sense.

I found a good example explaining the use of async()=> here.

@anuragvohraec - Here is a tweak to your code that works for me:

function add(a: any,b: any,c: any): any{
	if(a+b>10){
		return c("My error",a+b);
	}else{
		return c(null, a+b);
	}
}

(async()=>{
    try{
        let t = await (async()=>{
            return add(4,5,(e: any, r: any)=>{
                if(e) throw e;
                return r;	
            });
        })();
        console.log(t);
    }catch(e){
        console.log("Yes! caught error from call back: ", e);
    }
})()

@kitsonk
Copy link
Contributor

kitsonk commented May 11, 2020

@anuragvohraec it is a TypeScript lexing issue. Upstream bug filed: microsoft/TypeScript#38483.

@anuragvohraec
Copy link

anuragvohraec commented May 12, 2020

@anuragvohraec - Pardon my N00b question, but when would you use the (async(){...})() in real world logic in Deno?

The real world example for this will be to convert a call back based asynchronous function be converted to async await based without using a Promise API.

If you see my example:

function add(a: any,b: any,c: any): any{
	if(a+b>10){
		return c("My error",a+b);
	}else{
		return c(null, a+b);
	}
}

try{

 let t = await (async()=>{
	return add(4,5,(e: any, r: any)=>{
		if(e) throw e;
		return r;	
	});
 })();
 console.log(t);
}catch(e){
console.log("Yes! caught error from call back: ", e);
}

c is a callback function.
I was trying to use a IIFE to convert my call back back based asychronous function to an async-await based function (without using an explicit Promise API).

@anuragvohraec
Copy link

No, not all functions in are implicitly async. Only the global scope (so the top level of a module outside of a function scope) and all function scopes declared with the async keyword.

await foo() // valid here

function test1() {
  await foo() // invalid here
}

async function test2() {
  await foo() // valid here
}

The code I have shared do not commits any mistake mentioned in your code samples.

@kitsonk
Copy link
Contributor

kitsonk commented May 12, 2020

The code I have shared do not commits any mistake mentioned in your code samples.

@anuragvohraec I don't think @lucacasonato was complaining about your code, but was just trying to inform @JavaScriptDude on how async/await works. Your code is valid JavaScript, and should be valid TypeScript but TypeScript has a bug.

@somombo
Copy link

somombo commented May 14, 2020

is TLA supported in deno REPL?

I cant seem to get the following to work.

using Deno v.1.0.0

$ deno
> console.log(await Promise.resolve('Success'))

It just seems to await indefinitely without outputting anything or allowing me to input any further commands.

@josephrocca
Copy link
Contributor

josephrocca commented Aug 18, 2020

@somombo I believe #3700 is tracking the addition of top-level await to the REPL. For me (1.2.0 and 1.3.0) using top-level await in the REPL throws an error:

Uncaught SyntaxError: await is only valid in async function
    at evaluate (rt/40_repl.js:60:36)
    at replLoop (rt/40_repl.js:160:15)

hardfist pushed a commit to hardfist/deno that referenced this issue Aug 7, 2024
…nd#471)

Due to V8's PKU feature, only the thread that initialized the V8
platform or those spawned from it can access the isolates; other threads
will get a segmentation fault. Since each test runs in its thread, they
crash on newer CPUs that support PKU.

This issue has two possible solutions:
1. Initialize the platform once in the main thread.
2. Turn off the PKU feature during testing.

This issue was already encountered for the `deno test` command, which
was resolved by initializing the platform in the main thread
(denoland#20495).

In the case of rust tests, the same solution cannot be used, as the
default test runner does not allow running code on the main thread
before any test runs.

My proposed solution is to add a feature flag to deno_core, which, when
enabled, uses the unprotected platform. An alternative solution could be
to call `deno_core::JsRuntime::init_platform` at the beginning of each
test.

This PR will also be a step towards fixing denoland#21439
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.

10 participants