Skip to content
This repository has been archived by the owner on Sep 13, 2023. It is now read-only.

Add support for await #10

Open
saurabhdaware opened this issue May 22, 2020 · 12 comments
Open

Add support for await #10

saurabhdaware opened this issue May 22, 2020 · 12 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@saurabhdaware
Copy link
Member

We should be able to render something like {{ await myAsyncWork() }}. Currently, it throws an error saying await should be inside async. Even if we add async the value it will return will be a promise only but to print the content in HTML file we need that value to be the actual value instead of promise.

@saurabhdaware saurabhdaware added bug Something isn't working help wanted Extra attention is needed labels May 22, 2020
@mojiwa
Copy link

mojiwa commented May 29, 2020

Hi.
Could you give an example of where in the code you're trying to implement this, and what exactly you're doing it when it fails? Thanks

@saurabhdaware
Copy link
Member Author

saurabhdaware commented May 30, 2020

Hi there, Yes I should've added steps to reproduce already.

Here are the steps to reproduce:

  1. Create an example.abell file in any folder.
  2. Add this content to that file.
<html>
  <body>
  {{ await (await fetch('https://reqres.in/api/users?page=2')).json() }}
   </body>
</html>
  1. Run npx abell-renderer build --input example.abell

Currently, this will throw an error because in render when we execute this JS, we execute it on top level. This is the line of code that executes the code

https://github.com/abelljs/abell-renderer/blob/master/src/execute.js#L60

As you can see we're assigning it to a variable called aBellSpecificVariable and return what is there inside that variable.

Instead the above snippet should return the json and not throw error.

@mojiwa
Copy link

mojiwa commented May 30, 2020

Hi there, Yes I should've added steps to reproduce already.

Here are the steps to reproduce:

  1. Create an example.abell file in any folder.
  2. Add this content to that file.
<html>
  <body>
  {{ await (await fetch('https://reqres.in/api/users?page=2')).json() }}
   </body>
</html>
  1. Run npx abell-renderer build --input example.abell

Currently, this will throw an error because in render when we execute this JS, we execute it on top level. This is the line of code that executes the code https://github.com/abelljs/abell-renderer/blob/master/src/execute.js#L60. As you can see we're assigning it to a variable called aBellSpecificVariable and return what is there inside that variable.

Instead the above snippet should return the json and not throw error.

Thanks. I'll take a proper look later today. I think I have an idea of what's going on, but I'd rather check first and be sure.

@mojiwa
Copy link

mojiwa commented May 31, 2020

Hi there, Yes I should've added steps to reproduce already.

Here are the steps to reproduce:

  1. Create an example.abell file in any folder.
  2. Add this content to that file.
<html>
  <body>
  {{ await (await fetch('https://reqres.in/api/users?page=2')).json() }}
   </body>
</html>
  1. Run npx abell-renderer build --input example.abell

Currently, this will throw an error because in render when we execute this JS, we execute it on top level. This is the line of code that executes the code https://github.com/abelljs/abell-renderer/blob/master/src/execute.js#L60. As you can see we're assigning it to a variable called aBellSpecificVariable and return what is there inside that variable.

Instead the above snippet should return the json and not throw error.

So I couldn't get your example to work.
First I got the error: missing ) after argument list

Then I realised I needed to both install node-fetch and then add the following line to the .abell file:
{{ const fetch = require("node-fetch") }}
and then run the command with --allow-require

and now I get the following error:
Cannot find module 'node-fetch'

This is my entire file

{{ const fetch = require("node-fetch") }}

<html>
    <body>
        {{ await (await fetch('https://reqres.in/api/users?page=2')).json() }}
    </body>
</html>

Any ideas why I can't get the example to exit with the correct error?

@saurabhdaware
Copy link
Member Author

Oh yes you will have to npm install --save-dev node-fetch. With this, you will be able to require it in files.

@saurabhdaware
Copy link
Member Author

I made this sandbox, https://codesandbox.io/s/abell-renderer-await-example-w6t9v?file=/src/index.abell

You can run npm run build in custom terminal there

@mojiwa
Copy link

mojiwa commented Jun 1, 2020

I made this sandbox, https://codesandbox.io/s/abell-renderer-await-example-w6t9v?file=/src/index.abell

You can run npm run build in custom terminal there

Okay, so I spent a while having a real hack around with the code here.
I know exactly what's wrong, but implementing a solution is going to take a lot longer than I'd hoped.

So what's happening is that the vm is running the code just fine, but obviously it's just running everything synchronously. So at the point where you await the fetch request it's value is a promise (in a pending state).
To get the vm to wait until the request is completed before it returns a result requires some customisation as async/await isn't included out of the box for the vm.

Now, I will point out that this solution doesn't work - as in, it does not render the html as you'd like. The reason for that is that a proper refactor of the code is required to make it support async/await correctly.
But this 'hack' will let you see in the console that the vm implementation I've used does work, and hopefully can provide you with a basis on which to make the necessary changes.

Firstly in execute.js, I created a new async execute function. Within there we create a promise and once the VM has done its work, we consider the promised resolved. We can await that promise and only then return the result once fulfilled:

async function executeAsync(jsToExecute, sandbox= {}) {
  const numOfKeysBeforeExecution = Object.keys(sandbox).length;
  const codeToExecute = 
    'aBellSpecificVariable = ' + 
    jsToExecute.replace(/(?:const |let |var )/g, '');

  sandbox.value = "0";
  const result = await new Promise(resolve => {
    sandbox.resolve = resolve;
    const code = '(async() => await '+jsToExecute+'.then(async (result) => { value = await result.json(); resolve();}))()';
    const script = new vm.Script(code);
    const context = new vm.createContext(sandbox);
    script.runInContext(context);        
  }).then(() => {
    // console.log(sandbox.value);
    return {
      type: 'value',
      value: sandbox.value,
      sandbox        
    }
  });    
  return result;
}

Then in index.js I implemented the following at line 76 to intercept when a user uses 'await' in their code, and make it redirect to my executeAsync method:

// Executes the expression value in the sandbox environment
      if (match[1].includes('await')) {
        (async () => { await executeAsync(match[1], sandbox).then((result) => {   
          console.log(result.value);
        }); })();      
      } else {        
        const executionInfo = execute(match[1], sandbox); 
        if (executionInfo.type === 'assignment') { ...

The console output will give you the actual result from the fetch.

What needs to be done from this stage is something akin to the following:

  • It would be easier to make all of execute() async and have it return the data in the same way so that the render method doesn't need to call two different Execute methods.
  • render() needs to be made async so that the html can be generated only once the response comes back.
  • generateHTMLFromAbell() probably also needs to be made async so that it can await the rendering of the html template before it writes it.
  • You will probably need to rethink execution time as well, as it will need to take into account the time taken for the await requests to come back.

Anyway, I'm sorry I couldn't help implement a full solution, but it did end up taking a lot longer than I anticipated. Hopefully though this puts you on track for a proper solution.

@saurabhdaware
Copy link
Member Author

THAT'S A HUGE HELP @mojiwa !! Thank you so much 🎉 I will try out your snippet probably by tomorrow or the day after.

Again thank you so much!

@mojiwa
Copy link

mojiwa commented Jun 2, 2020

THAT'S A HUGE HELP @mojiwa !! Thank you so much 🎉 I will try out your snippet probably by tomorrow or the day after.

Again thank you so much!

No problem. I'm glad I could help.

@pantharshit00
Copy link
Member

Node is also releasing support for top level await. That might be relevant here

@pantharshit00
Copy link
Member

@pantharshit00
Copy link
Member

This is the next thing that I will pick up. Let me know if you have another idea other than the above discussion

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants