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

openExternalLink returns a { opened: boolean } result #282

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

HamzaAtDiscord
Copy link

Edit the return type for openExternalLink. With this change the result of openExternalLink will be an object containing a boolean value opened.

If opened is true, then the link was opened by the user
If opened is false, then the link was not opened by the user

The lifetime of the promise is also changed. Previously the promise completed after opening the "Leaving Discord" dialog, but with this change the promise completes after the user selects an action in the dialog (visit the site, or go back). If the dialog does not appear because the site visit is automatically approved, then the promise returns { opened: true } following the link opening.

@HamzaAtDiscord HamzaAtDiscord force-pushed the hamzahutchinson/change-open-external-link-response branch from b030381 to 2faa932 Compare November 15, 2024 01:36
Comment on lines +109 to +112
export const OpenExternalLinkResponse = zod.object({
opened: zod.boolean(),
});

Copy link

Choose a reason for hiding this comment

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

general question: how does this behave against a version of the client that doesn't send the opened flag across RPC? my gut is that since this isn't marked as optional and we're using .parse() it would error out, but I also don't know if we have error handling for that buried elsewhere (I'm assuming we do since otherwise I imagine this would have been a problem already).

@@ -1,6 +1,6 @@
{
"name": "@discord/embedded-app-sdk",
"version": "1.7.0",
"version": "1.7.1",
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit - we don't bump the package version in the PR, it happens automatically via the release please github action.

nit - I think this is a feature, not a patch

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