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

Revert "feat: add alert, confirm, and prompt (#7507)" #7959

Closed
wants to merge 1 commit into from

Conversation

bartlomieju
Copy link
Member

@bartlomieju bartlomieju commented Oct 13, 2020

This reverts commit 0dcaea7.

Revert #7507 - only after landing I noticed controversy in the original issue (#4257).

CC @kt3k @kitsonk

@kt3k
Copy link
Member

kt3k commented Oct 13, 2020

Security concern doesn't apply anymore because the same attack can be done with exiting APIs (such as Deno.stdin.readSync).

I agree with his second argument ( there is no reasonable way to use alert confirm prompt in a isomorphic way), but I believe this is not strongly against the addition of APIs.

I'd like @kitsonk to review my comment at #4257 (comment). My point (motivation) of this addition is that these are very useful for the beginners and learners. Maybe nobody use these in serious / production environment, but these helps learners easily interact with Deno APIs in casual manner (and I hope that promote the use of Deno especially for newcomers)

@kitsonk
Copy link
Contributor

kitsonk commented Oct 13, 2020

I am not a fan of this addition, but if I had a real problem with it, I would have spoken up in the PR. It doesn't "break" Deno, so I would rather have arguments about more serious things! 😉

@bartlomieju
Copy link
Member Author

My views are in line with @kt3k opinion, closing without merge.

@bartlomieju bartlomieju deleted the revert_7507 branch October 13, 2020 19:52
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