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

invoke async function as IIFE in example #958

Closed
wants to merge 1 commit into from
Closed

invoke async function as IIFE in example #958

wants to merge 1 commit into from

Conversation

avindra
Copy link

@avindra avindra commented Dec 11, 2019

The example as listed currently does not run anything, so wrapping it up as an async IIFE.

What this does:

Make sure users who are copy+pasting the example in the README can get it to run

The example as listed currently does not run anything, so wrapping it up as an async IIFE.
@dhensby
Copy link
Collaborator

dhensby commented Dec 13, 2019

Thanks for the contribution @avindra

I'm not entirely sure this change makes the docs any more helpful - it's not intended to be a copy+paste example of getting a query to run, it's meant to be an abstract example of how you would go about writing code. I think putting the example code in an IIFE will actually make more people's code break as they'll copy+paste it into their functions and not understand why it's running when the function hasn't been called.

@willmorgan
Copy link
Collaborator

why it's running when the function hasn't been called

It's been called in the IIFE!

@dhensby
Copy link
Collaborator

dhensby commented Dec 13, 2019

It's been called in the IIFE!

Yes, but is someone blindingly copy and pasting code going to understand that? these examples are not for blind copy and paste to get your application working, they are conceptual examples about how to make a request - if you can't figure out you need to actually call that code for it to run, I don't think they'll understand why it's running inside an IIFE

@dhensby
Copy link
Collaborator

dhensby commented Dec 13, 2019

If we do think this is how our docs should be written then every single example needs updating, right?

@avindra
Copy link
Author

avindra commented Dec 13, 2019

Fair points, gents. I was able to get up and running from the example, so I'll leave it be for now.

As you mentioned @dhensby everything else should get updated if this is the approach.

I'm closing this for now... may re-open it later on if I get some time to update the rest of the docs

@avindra avindra closed this Dec 13, 2019
@dhensby
Copy link
Collaborator

dhensby commented Apr 2, 2020

@dhensby dhensby mentioned this pull request Jul 22, 2020
@avindra
Copy link
Author

avindra commented Oct 2, 2020

@dhensby good points are raised in that anti-iife article. Top level await is available (behind a flag) in node 14.3.0 and is already in deno. At some point the function can be removed, and the top level await should work as expected.

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.

3 participants