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

Use asynchronous GET requests / Replace xmlhttprequest-ts with fetch API for module loading #1348

Closed
Yongbeom-Kim opened this issue Feb 1, 2023 · 3 comments · Fixed by #1637
Assignees
Labels
_help wanted Extra attention is needed important Fixing this is important, but not mission-critical

Comments

@Yongbeom-Kim
Copy link

Yongbeom-Kim commented Feb 1, 2023

Continued from #1340

Problem

  • Currently, js-slang uses the xmlhttprequest-ts package to make GET requests to retrieve module data.
  • The package has not had any updates/releases in the past 5 years, and it seems safe to say that this package should be deprecated in favour of something like fetch instead, as suggested by a contributor(?) Retire project, and archive it. hmoog/xmlhttprequest-ts#6
  • It's also worthy of noting that synchronous/blocking XMLHttpRequests (as is being done in js-slang) are deprecated
  • Well, it makes sense that the get request should be asynchronous/non-blocking?

Proposal

  • Refactor js-slang to load modules asynchronously (with async functions and promises)
  • Replace all XMLHttpRequest GET requests with fetch requests
  • Use top-level await to load modules (if needed)
    • I don't think this is a big issue, noting that top-level await is supported on all browsers
    • This also requires bumping tsconfig.json to:
{
  "compilerOptions": {
    "module": "esnext" (or system),
    "target": "es2017",
}
@leeyi45
Copy link
Contributor

leeyi45 commented Apr 9, 2023

As part of this, I'm thinking we delegate loading of tabs to the frontend. Tabs are a frontend only concept, so I don't think it makes sense for js-slang to do the loading.

Related to this issue

I've done some looking into this, most of the runners aren't affected, except maybe the interpreter, not sure if that one supports async evaluation

@RichDom2185 RichDom2185 added _help wanted Extra attention is needed important Fixing this is important, but not mission-critical labels Nov 16, 2023
@RichDom2185
Copy link
Member

I would say this is an issue of high enough priority; I will pin it to the top of our issues list.

@RichDom2185 RichDom2185 pinned this issue Nov 22, 2023
@leeyi45
Copy link
Contributor

leeyi45 commented Nov 30, 2023

This transition is actually almost complete. Most of the features surrounding module loading are asynchronous, namely

  • Loading the module manifest
  • Module Documentation
  • Transpiler
  • Stepper

The two remaining areas of concern are:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
_help wanted Extra attention is needed important Fixing this is important, but not mission-critical
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants