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

Top-level await #87

Closed
blixt opened this issue Sep 26, 2021 · 12 comments
Closed

Top-level await #87

blixt opened this issue Sep 26, 2021 · 12 comments

Comments

@blixt
Copy link

blixt commented Sep 26, 2021

Top-level await doesn't seem to parse. This code:

const response = await fetch("https://google.com");

Fails with this error:

unexpected fetch in const declaration on line 1 and column 24
    1: const response = await fetch("https://google.com");

Playground: https://play.golang.org/p/vBfz3yFM3J9

@tdewolff
Copy link
Owner

tdewolff commented Sep 26, 2021

Yes, because that is actually invalid JS, see https://eslint.org/demo using your snippet. It returns Parsing error: Unexpected token fetch or Parsing error: Cannot use keyword 'await' outside an async function when parsing as a module.

@blixt
Copy link
Author

blixt commented Sep 26, 2021

It might be invalid according to ESLint's configuration, but it's definitely valid. Run this in Chrome's console and you'll see it chugging along just fine:

const js = `
  const response = await fetch("https://api.coindesk.com/v1/bpi/currentprice.json");
  const data = await response.json();
  export { data }
`
const url = await URL.createObjectURL(new Blob([js], { type: "text/javascript" }))
const module = await import(url)
console.log(module.data);

Screen Shot 2021-09-26 at 21 04 09


(Edit: Rewrote the example to be a bit more readable, but same result.)

@blixt
Copy link
Author

blixt commented Sep 26, 2021

Ah I see, there's a closed issue in ESLint from when top-level await was a stage 3 proposal but it's stage 4 now and should be supported by parsers, even if not every browser might have it yet: https://github.com/tc39/proposal-top-level-await#ecmascript-proposal-top-level-await

@blixt
Copy link
Author

blixt commented Sep 26, 2021

Okay dug into it some more, ESLint 8 has support for it, but it's not fully released yet (a release candidate was made two days ago).

@blixt
Copy link
Author

blixt commented Sep 26, 2021

But multiple browsers and the V8 engine that Node.js runs on under the hood support it so that alone seems to be a good reason to support it? At least I'm using this library for real production cases where top-level await might appear in the code.

Also, "Run this in Chrome's console" in general is a terrible test, as Chrome is notorious for essentially writing their own standards.

Maybe, though I don't think that's a discussion for this issue. Top-level await is an official TC39 ECMAScript proposal that has been fully finished (no longer a proposal but a standard) which means it's not Chrome making up their own standard. Also Safari 15 and Firefox support this.

(Edit: I ran the same code in Firefox and Safari and it runs equally well in both. Note that Safari console doesn't support await so you need to wrap the code for the console in (async function () { … })() but this is just for the console to run the code as expected – ES Modules still support top-level await in Safari).

@blixt
Copy link
Author

blixt commented Sep 26, 2021

Ah, turns out Node.js 14 supports this, so I don't even think the original argument for why it's not needed is valid. The reason your code didn't run is because you're trying to call fetch (a browser API) in Node.js. Try a custom async function instead and it should work.

@tdewolff
Copy link
Owner

I think we should support any proposal that is (close to) being integrated into the spec. This package should not be a blocker for upcoming features when used in production. The relevant implementation is fairly simple, see https://tc39.es/proposal-top-level-await/#sec-modules, and I'll try to get to it soon.

@blixt
Copy link
Author

blixt commented Sep 26, 2021

@89z sorry but this is a standardized feature. All major browsers support it. So does Node.js since v14. Check your original comment and you'll see the error is not about await but about fetch. Sorry, but your comments have not been very productive to this issue since the points you're making are not correct, they're verifiably incorrect.

Try this in Node.js instead:

const myPromise = new Promise(resolve => setTimeout(() => resolve("hello"), 1000))
export default await myPromise

@blixt
Copy link
Author

blixt commented Sep 26, 2021

Just because you say its a standardized feature, doesnt make it standardized feature. Look, I can say "the sky is green", but thats not actually true. The link provided by @tdewolff [1], appears to say "proposal", which would directly contradict the point youre trying to make.

I'm not just saying it's a standardized proposal. That link is outdated. I already linked to the more up-to-date source for the same TC39 team saying it's Stage 4, finished, a standard. I also invite you to read the most up-to-date source for the ECMAScript standard – https://tc39.es/ecma262/ – which references top-level await as well. In the introduction it also links which standards it includes: https://github.com/tc39/proposals/blob/HEAD/finished-proposals.md and there you will also find top-level await.

I am only responding to you in the hopes that it inspires less reactionary comments based on hunches, and that you research your statements in the hopes that we can all agree much sooner on what is the actual state of things.

so if you didnt want people using it, then maybe you shouldnt have posted it.

I explicitly stated to run it in a browser console. I said "Chrome" though "Firefox" or "Safari 15" are valid substitutes. I am sure as a Node.js developer choosing to run browser code in Node.js you are aware of the API differences and in choosing to do so (even if I never asked anyone to run this in Node.js), you would be able to substitute the code for a suitable asynchronous API in Node.js. So I don't understand why you're telling me not to post it.

Same to you.

See the top of this comment with the correct information. I can only repeat once more what I said before.

@blixt
Copy link
Author

blixt commented Sep 26, 2021

Okay, maybe I'm wrong in thinking that while the standard is snapshotted yearly it's a running standard and that things that run the full process into the specification will never get taken out of the specification without another proposal process. Please let me know if that's the case because then I've been misinterpreting how web standards work.

I also concede that I could have written an example that works in any platform and not just browser platforms.

I think that's it for me in this issue, I think we already added too much noise for Taco, that isn't really productive with regards to the original issue or the next action items.

@blixt
Copy link
Author

blixt commented Sep 26, 2021

Fair enough, I apologize for approaching it in such a stubborn way. It is a feature I've tracked for a long time and now that it's in the spec (draft) and all major JS platforms I got a bit passionate about it. I'll take care to report issues with a little more nuance since me arguing about what is a spec isn't really productive, while getting to an agreement to support the feature is.

@tdewolff
Copy link
Owner

tdewolff commented Oct 4, 2021

The proposal is already in ECMA2022 (the latest spec document at https://tc39.es/ecma262) which this library follows. So I've implemented the proposal, please let me know if you bump into issues!

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

No branches or pull requests

2 participants