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

Javascript parser does not support top-level await #7408

Closed
headbank opened this issue May 27, 2024 · 6 comments · Fixed by #7417
Closed

Javascript parser does not support top-level await #7408

headbank opened this issue May 27, 2024 · 6 comments · Fixed by #7417
Assignees
Labels
JavaScript [ci] enable web job and extra JavaScript tests (webcommon/javascript2.editor) kind:feature A feature request

Comments

@headbank
Copy link

Apache NetBeans version

Apache NetBeans 21

What happened

It's now possible to use await in top-level code in JavaScript module context (which may be a normal JS file loaded via dynamic include() call). However NetBeans's Javascript parser interprets such a statement as an error.

Language / Project Type / NetBeans Component

Editor Javascript parser (PHP project)

How to reproduce

  1. Create a file with .js extension
  2. Write a top-level await statement in the file, e.g.
await Promise.resolve(1);

Did this work correctly in an earlier version?

No / Don't know

Operating System

Gentoo Linux amd64

JDK

openjdk-21.0.3_p9 Prebuilt Java JDK binaries provided by Eclipse Temurin

Apache NetBeans packaging

Apache NetBeans binary zip

Anything else

No response

Are you willing to submit a pull request?

No

@headbank headbank added kind:bug Bug report or fix needs:triage Requires attention from one of the committers labels May 27, 2024
@troizet troizet added the JavaScript [ci] enable web job and extra JavaScript tests (webcommon/javascript2.editor) label May 27, 2024
@matthiasblaesing matthiasblaesing added kind:feature A feature request and removed kind:bug Bug report or fix needs:triage Requires attention from one of the committers labels May 28, 2024
@matthiasblaesing matthiasblaesing self-assigned this May 28, 2024
matthiasblaesing added a commit to matthiasblaesing/netbeans that referenced this issue May 28, 2024
matthiasblaesing added a commit to matthiasblaesing/netbeans that referenced this issue May 29, 2024
@matthiasblaesing
Copy link
Contributor

Please check/test #7417. There is also a nightly build available from the check page or directly: https://github.com/apache/netbeans/suites/24280981849/artifacts/1549006976

@headbank
Copy link
Author

Thanks, the dev build improves the situation but it is still incomplete as it makes the validity of the await statement dependent on there also being an export statement in the file, which while recommended practice, is not a language requirement.

@matthiasblaesing
Copy link
Contributor

Thanks, the dev build improves the situation but it is still incomplete as it makes the validity of the await statement dependent on there also being an export statement in the file, which while recommended practice, is not a language requirement.

The language requirement is, that top level await is allowed for modules. The NB implementation currently guesses whether the file should be parsed as a module based on the import and export keywords. A work around might be to add support for the mjs extension and let these file be parsed as modules by default. What do you think?

But before doing this: Is there a real world example for JS modules without export/import?

@headbank
Copy link
Author

I'll start by saying I may be guilty of mistaking "Firefox doesn't prevent you doing it" for "valid" in terms of the language. I've been going off the MDN pages on await and import() and have been uncertain how rigidly-defined modules are at this point in language terms.

Regarding .mjs I note the "Aside" on the subject in the MDN Modules page and think it is probably not (yet) a good determinant as it may be impractical for people to deploy with.

The only real-world example I can speak on is my own codebase, which I'm in the midst of rewriting (thus how I washed up here). Some time ago I adapted my existing class-loader code to employ dynamic import() due to the advantages of it being Promise-based and that it prevents repeat-downloading of a URI; but I maintained the actual class files as already written, which is that they declare themselves in global scope:

window.MyClass = class MyClass {...};

Obviously this is a side-effect, which modules are discouraged from having, but not using export kept the door open to being able to load them by other means if desired.

It's useful that import() can be employed in a non-module context, and upon files that bear no module-specific syntax; likewise that top-level await does not place any module-specific syntax requirements on its file either. However it's fairly clear such usage is not intended so I'd understand if you demur on supporting it.

Would a warning, rather than error, condition be a compromise worth considering? (In context where export or import are absent)

matthiasblaesing added a commit to matthiasblaesing/netbeans that referenced this issue Jun 2, 2024
@matthiasblaesing
Copy link
Contributor

I looked a bit longer into this and my take on this that it can't be done without very ugly compromises. I tried to add a variant to the parset, that allows callers to allow top-level-await also in script context. However, that breaks in subtle ways. consider this:

var await = 3;

If parsed as a module this is invalid code as the keyword await can not be used as an identifier, while in script mode that is totally fine.

The definition can be found a bit more wordy here (Note 1):

https://tc39.es/ecma262/#sec-async-function-definitions

The TL;DR version is, that you can't parse a JS file without knowing how it is going to be used, because grammar depends on that. That becomes even more fun, when the same file is used as script and module.

Using different extensions for scripts and modules as introduced by node looks like a sane decision to escape this dilemma, which I added to the PR.

@headbank
Copy link
Author

headbank commented Jun 2, 2024

Blimey. I'd never have expected that the above example would be permissible in any context! Makes no sense to me, but the spec per your link is clear enough.

Given that the proper await usage has syntax incompatible with any way in which it could be used as an identifier (that I can think of), the context-based distinction here seems artificial and non-useful because parsers can differentiate. It should be a keyword in either context, or neither, if I had a vote.

Anyway I can't fault your conclusion and thanks for your work on this. I've benefited from this "looseness" in the definition of a module up til now, but I do hope it gets tightened up a little (based on extension does seem a sensible method) eventually so the whole ecosystem can coalesce on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JavaScript [ci] enable web job and extra JavaScript tests (webcommon/javascript2.editor) kind:feature A feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants