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

feat: adding globalAwait option #102

Closed
wants to merge 1 commit into from
Closed

feat: adding globalAwait option #102

wants to merge 1 commit into from

Conversation

SilentVoid13
Copy link

Implements feature of #99.

This feature allows us to add all data objects in an array and to await for it globally (only when the globalAwait option is set to true in the config).

This adds almost no overhead at runtime (adding a for loop to generate the array push, but that's neglectable overhead), because Promise.all just returns the value of synchronous functions and variables and only awaits for asynchronous functions. This could even be the default behavior when resolving templates (removing the option).

This allows us to speed things up a lot when having a lot of asynchronous functions in our data object.

Because this doesn't change anything about how this template engine works and just adds some extra speed, I don't think this is an out of scope feature, it is very useful when dealing with asynchronous functions.

@ghost
Copy link

ghost commented Apr 2, 2021

Congratulations 🎉. DeepCode analyzed your code in 5.62 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard | Configure the bot

@SilentVoid13 SilentVoid13 reopened this Apr 2, 2021
@shadowtime2000
Copy link
Collaborator

Sorry for taking so long, I will take a look at this today.

@@ -26,6 +26,7 @@ export default function compileToString(str: string, config: EtaConfig): string
(config.include ? ',include=E.include.bind(E)' : '') +
(config.includeFile ? ',includeFile=E.includeFile.bind(E)' : '') +
'\nfunction layout(p,d){__l=p;__lP=d}\n' +
(config.globalAwait ? 'let prs = [];\n' : '') +
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you think we could rename this to _prs? That is something I have seen many tools which need to add additional variables use so it doesn't conflict with the actual code.

Copy link
Collaborator

@shadowtime2000 shadowtime2000 left a comment

Choose a reason for hiding this comment

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

Can you format this with prettier/eslint?

@shadowtime2000 shadowtime2000 linked an issue Apr 5, 2021 that may be closed by this pull request
This option adds all data objects into an array and await for it
globally.
@SilentVoid13
Copy link
Author

@shadowtime2000 This should be good !

Copy link
Collaborator

@shadowtime2000 shadowtime2000 left a comment

Choose a reason for hiding this comment

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

I am still trying to figure out how exactly this would be tested, but if you know a way could you test this?

@SilentVoid13
Copy link
Author

Hey @shadowtime2000, sorry for the late response.
I can't really think of a good way to test as I'm not familiar with unit tests in JS / TS.
We're really just adding everything in an array and awaiting for it so I don't think we need any new fancy tests compared to the existing ones.

@shadowtime2000
Copy link
Collaborator

@SilentVoid13 Sorry for the late response.

Sorry, I am a little scared of merging something like this which is kind of hard to test so I am going to put this on hold I guess. Sorry again.

blutorange added a commit to blutorange/eta that referenced this pull request Nov 28, 2021
blutorange added a commit to blutorange/eta that referenced this pull request Nov 28, 2021
@nebrelbug
Copy link
Collaborator

I'm going to close this as stale and because it's for Eta v2. I'd be open to accepting a pull request with this functionality for v3.

@nebrelbug nebrelbug closed this Jun 13, 2023
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

Successfully merging this pull request may close these issues.

Globally async data object and global await feature
3 participants