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.

Sign up for GitHub

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

Jump to bottom

asyncjs: don't use JavaScript async #1359

Open
wants to merge 2 commits into
base: devel
Choose a base branch
from
Open

asyncjs: don't use JavaScript async #1359

wants to merge 2 commits into from

Conversation

zerbina
Copy link
Collaborator

@zerbina zerbina commented Jun 26, 2024

Summary

  • asyncjs no longer uses JavaScript's async
  • result in .async procedures now works like it does everywhere else
  • methods can no longer use .async

Details

  • the goal is removing the usage of the deprecated .codegenDecl pragma
  • routines marked with the .async pragma internally use a closure
    iterator that is then chained to a promise object when awaiting

To-Do

  • make imported exceptions properly catchable within closure iterators (separate PR)
  • re-add support for returning Futures from .async procedures
  • write a proper commit message

Notes for Reviewers

  • prior discussion about this can be found here
  • an important prerequisite for the new JavaScript code generator

@zerbina zerbina added the stdlib Standard library label Jun 26, 2024
saem
saem approved these changes Jun 27, 2024
View reviewed changes
Copy link
Collaborator

@saem saem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor doc suggestion

lib/js/asyncjs.nim
## |NimSkull| generates JavaScript code that uses the ``Promise`` and ``Error``
## APIs, both which are supported by most modern versions of browsers, Node.js
## and Electron.
##
## If you need to use this module with older versions of JavaScript, you can
## use a tool that backports the resulting JavaScript code, as babel.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
## use a tool that backports the resulting JavaScript code, as babel.
## use a tool that backports the resulting JavaScript code, such as babel.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it's not strictly part of the change, but it was right there; not a requirement to update it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@saem saem saem approved these changes

Assignees
No one assigned
Labels
stdlib Standard library
Projects
None yet
Milestone
Standard library additions or cleanup
Development

Successfully merging this pull request may close these issues.
2 participants
@zerbina @saem