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

feat: openExternalLink returns a { opened: boolean } result #282

Merged
merged 4 commits into from
Nov 19, 2024
Merged

feat: openExternalLink returns a { opened: boolean } result #282

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2faa932
Add a response type for openExternalLink that returns a boolean opene…
HamzaAtDiscord Nov 14, 2024
73a6285
revert the package version change
HamzaAtDiscord Nov 18, 2024
13e5550
adding a fallback to OpenExternalLinkResponse for older clients
HamzaAtDiscord Nov 18, 2024
0bdbc6c
fix: linting fix
HamzaAtDiscord Nov 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@discord/embedded-app-sdk",
"version": "1.7.0",
"version": "1.7.1",
Copy link
Contributor

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, I didn't realize! I have reverted to 1.7.0

"description": "@discord/embedded-app-sdk enables you to build rich, multiplayer experiences inside Discord.",
"author": "Discord",
"license": "MIT",
Expand Down
8 changes: 6 additions & 2 deletions src/commands/openExternalLink.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {Commands} from '../schema/common';
import {EmptyResponse} from '../schema/responses';
import {OpenExternalLinkResponse} from '../schema/responses';
import {TSendCommand} from '../schema/types';
import {commandFactory} from '../utils/commandFactory';

Expand All @@ -11,4 +11,8 @@ export interface OpenExternalLinkInput {
*
*/
export const openExternalLink = (sendCommand: TSendCommand) =>
commandFactory<OpenExternalLinkInput, typeof EmptyResponse>(sendCommand, Commands.OPEN_EXTERNAL_LINK, EmptyResponse);
commandFactory<OpenExternalLinkInput, typeof OpenExternalLinkResponse>(
sendCommand,
Commands.OPEN_EXTERNAL_LINK,
OpenExternalLinkResponse,
);
2 changes: 1 addition & 1 deletion src/mock.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ export const commandsMockDefault: IDiscordSDK['commands'] = {
startPurchase: () => Promise.resolve([]),
setConfig: () => Promise.resolve({use_interactive_pip: false}),
userSettingsGetLocale: () => Promise.resolve({locale: ''}),
openExternalLink: () => Promise.resolve(null),
openExternalLink: () => Promise.resolve({opened: false}),
encourageHardwareAcceleration: () => Promise.resolve({enabled: true}),
captureLog: () => Promise.resolve(null),
setOrientationLockState: () => Promise.resolve(null),
Expand Down
7 changes: 6 additions & 1 deletion src/schema/responses.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ export const GetChannelPermissionsResponse = zod.object({
permissions: zod.bigint().or(zod.string()),
});

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).

Copy link
Collaborator

Choose a reason for hiding this comment

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

this is a good callout

Copy link
Member

Choose a reason for hiding this comment

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

would defaulting this to false be appropriate?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good call. I've updated this to use fallbackToDefault and return { opened: null } in the case where parsing fails (ie: discord clients that don't support this response). I've also updated the documentation with the null case and a note on when it occurs.

export {InitiateImageUploadResponseSchema as InitiateImageUploadResponse};

/**
Expand Down Expand Up @@ -162,8 +166,9 @@ function parseResponseData({cmd, data}: zod.infer<typeof ResponseFrame>) {
return SubscribeResponse.parse(data);
case Commands.USER_SETTINGS_GET_LOCALE:
return UserSettingsGetLocaleResponse.parse(data);
// Empty Responses
case Commands.OPEN_EXTERNAL_LINK:
return OpenExternalLinkResponse.parse(data);
// Empty Responses
case Commands.SET_ORIENTATION_LOCK_STATE:
case Commands.SET_CERTIFIED_DEVICES:
case Commands.SEND_ANALYTICS_EVENT:
Expand Down