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

suggestion: Deno.permissions.querySync #15576

Closed
iuioiua opened this issue Aug 24, 2022 · 6 comments · Fixed by #17019
Closed

suggestion: Deno.permissions.querySync #15576

iuioiua opened this issue Aug 24, 2022 · 6 comments · Fixed by #17019
Labels
permissions related to --allow-* flags public API related to "Deno" namespace in JS runtime Relates to code in the runtime crate suggestion suggestions for new features (yet to be agreed)

Comments

@iuioiua
Copy link
Contributor

iuioiua commented Aug 24, 2022

Deno.permissions.query is currently async. A sync version would be useful in contexts where top-level await would otherwise break node. Refs:

In denoland/std#1495 in particular, @bartlomieju mentions that a sync version of this API might not be a good idea. Is this still the case? If so, is there a decent workaround for the above-mentioned refs?
@lucacasonato lucacasonato added permissions related to --allow-* flags public API related to "Deno" namespace in JS suggestion suggestions for new features (yet to be agreed) runtime Relates to code in the runtime crate labels Aug 24, 2022
@iuioiua
Copy link
Contributor Author

iuioiua commented Sep 7, 2022

@bartlomieju, do you have any thoughts?

@iuioiua
Copy link
Contributor Author

iuioiua commented Oct 7, 2022

@bartlomieju, just following up. Would love to hear your thoughts.

@lino-levan
Copy link
Contributor

I remember reading somewhere that this was planned and then scrapped by the core team but I can't seem to find that discussion right now. @bartlomieju did that happen or am I misremembering it?

@martin-braun martin-braun mentioned this issue Nov 17, 2022
Closed
@martin-braun
Copy link

martin-braun commented Nov 17, 2022 •
edited
Loading

Solving this would help to finish denoland/std#2785, context here: denoland/std#2594 (comment)

Discussed this offline with ry, jumping to adding sync permissions API is not a good idea.

Originally posted by @bartlomieju in denoland/std#1495 (comment)

Could you please elaborate?

@martin-braun martin-braun mentioned this issue Nov 17, 2022
Merged
@iuioiua
Copy link
Contributor Author

iuioiua commented Dec 7, 2022 •
edited
Loading

@bartlomieju, are you able to elaborate? I might just give this a crack anyway.

@bartlomieju
Copy link
Member

We have recently discussed it and @kt3k suggested that it might be needed in deno_std. I'm still in favor of adding sync version of permission APIs, if you want to give it a crack a PR would be good to discuss it further 👍

@iuioiua iuioiua mentioned this issue Dec 12, 2022
Merged
rivy added a commit to rivy-js/yargs that referenced this issue Oct 30, 2023
@rivy
feat(deno): remove (now unneeded) top-level-await (requires Deno v1.3…
0a53704 
…0.0+)

- `Deno.permissions.querySync()` added in Deno v1.30.0 (on 2023-01-25)
- see [suggest: Deno.permissions.querySync](denoland/deno#15576)
- see [feat: Add sync APIs for "Deno.permissions"](denoland/deno#17019)
- see [v1.30.0](https://github.com/denoland/deno/releases/tag/v1.30.0) released on 2023-01-25
rivy added a commit to rivy-js/yargs that referenced this issue Oct 30, 2023
@rivy
feat(deno): remove unneeded top-level-await (requires Deno v1.30.0+)
2607dfd 
- `Deno.permissions.querySync()` added in Deno v1.30.0 (on 2023-01-25)
- see [suggest: Deno.permissions.querySync](denoland/deno#15576)
- see [feat: Add sync APIs for "Deno.permissions"](denoland/deno#17019)
- see [v1.30.0](https://github.com/denoland/deno/releases/tag/v1.30.0) released on 2023-01-25
rivy added a commit to rivy-js/yargs that referenced this issue Nov 20, 2023
@rivy
change/deno ~ remove unneeded top-level-await (requires Deno v1.30.0+)
fdb0b34 
- `Deno.permissions.querySync()` added in Deno v1.30.0 (on 2023-01-25)
- see [suggest: Deno.permissions.querySync](denoland/deno#15576)
- see [feat: Add sync APIs for "Deno.permissions"](denoland/deno#17019)
- see [v1.30.0](https://github.com/denoland/deno/releases/tag/v1.30.0) released on 2023-01-25
rivy added a commit to rivy-js/yargs that referenced this issue Nov 22, 2023
@rivy
change/deno ~ remove unneeded top-level-await (requires Deno v1.30.0+)
7437672 
- `Deno.permissions.querySync()` added in Deno v1.30.0 (on 2023-01-25)
- see [suggest: Deno.permissions.querySync](denoland/deno#15576)
- see [feat: Add sync APIs for "Deno.permissions"](denoland/deno#17019)
- see [v1.30.0](https://github.com/denoland/deno/releases/tag/v1.30.0) released on 2023-01-25
rivy added a commit to rivy-js/yargs that referenced this issue Nov 23, 2023
@rivy
change/deno ~ remove unneeded top-level-await (requires Deno v1.30.0+)
a8f492e 
- `Deno.permissions.querySync()` added in Deno v1.30.0 (on 2023-01-25)
- see [suggest: Deno.permissions.querySync](denoland/deno#15576)
- see [feat: Add sync APIs for "Deno.permissions"](denoland/deno#17019)
- see [v1.30.0](https://github.com/denoland/deno/releases/tag/v1.30.0) released on 2023-01-25
rivy added a commit to rivy-js/yargs that referenced this issue Nov 23, 2023
@rivy
change/deno ~ remove unneeded top-level-await (requires Deno v1.30.0+)
cbbde95 
- `Deno.permissions.querySync()` added in Deno v1.30.0 (on 2023-01-25)
- see [suggest: Deno.permissions.querySync](denoland/deno#15576)
- see [feat: Add sync APIs for "Deno.permissions"](denoland/deno#17019)
- see [v1.30.0](https://github.com/denoland/deno/releases/tag/v1.30.0) released on 2023-01-25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
permissions related to --allow-* flags public API related to "Deno" namespace in JS runtime Relates to code in the runtime crate suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: Add sync APIs for "Deno.permissions"
5 participants
@lucacasonato @lino-levan @bartlomieju @martin-braun @iuioiua