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

fix: Embed API Typings for 'on' and 'off' #16714

Merged
merged 2 commits into from
Sep 19, 2024

Conversation

hariombalhara
Copy link
Member

@hariombalhara hariombalhara commented Sep 19, 2024

What does this PR do?

  • Fixes the typescript type of argument of callback in 'on' and 'off'
    • Included playground.ts in tsconfig.json so that types are verified in it which verifies the fix as well.
  • Also exports EmbedEvent type so that it is easy to define a function separately to be assigned to callback. This is a requirement as well when using off

Before:
image
It was a union of all the events' payloads

Now:
image

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • N/A I have added a Docs issue here if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  • Attach a listener for any event(e.g. bookingSuccessfulV2 and verify the Typescript type of e.detail.data)
Cal("on", {
    action: "bookingSuccessfulV2",
    callback: (e) => {
      const data = e.detail.data;
      console.log("bookingSuccessfulV2", {
        endTime: data.endTime,
        startTime: data.startTime,
        title: data.title,
      });
    },
  });

Checklist

  • I haven't read the contributing guide
  • My code doesn't follow the style guidelines of this project
  • I haven't commented my code, particularly in hard-to-understand areas
  • I haven't checked if my changes generate no new warnings

@keithwillcode keithwillcode added core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO labels Sep 19, 2024
Copy link

vercel bot commented Sep 19, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Visit Preview Sep 19, 2024 9:25am
calcom-web-canary ⬜️ Ignored (Inspect) Visit Preview Sep 19, 2024 9:25am

@@ -1,7 +1,8 @@
import type { GlobalCal } from "./src/embed";

const Cal = window.Cal as GlobalCal;
const callback = function (e) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const callback = function (e: any) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This callback is used by all types of events so we use any

Comment on lines -145 to -146
// TODO: This makes api("on", {}) loose it's generic type. Find a way to fix it.
// e.g. api("on", { action: "__dimensionChanged", callback: (e) => { /* `e.detail.data` has all possible values for all events/actions */} });
Copy link
Member Author

Choose a reason for hiding this comment

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

This is now fixed.

@hariombalhara hariombalhara marked this pull request as ready for review September 19, 2024 08:24
@graphite-app graphite-app bot requested a review from a team September 19, 2024 08:24
@dosubot dosubot bot added the embed area: embed, widget, react embed label Sep 19, 2024
Copy link

graphite-app bot commented Sep 19, 2024

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (09/19/24)

1 reviewer was added to this PR based on Keith Williams's automation.

Copy link
Member

@sean-brydon sean-brydon left a comment

Choose a reason for hiding this comment

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

LGTM - this works but im not 100% sure how lol some TS magic going on here

@sean-brydon sean-brydon enabled auto-merge (squash) September 19, 2024 09:05
@sean-brydon sean-brydon merged commit 6d133c2 into main Sep 19, 2024
38 checks passed
@sean-brydon sean-brydon deleted the fix-callback-argument-type-embed branch September 19, 2024 09:42
Copy link
Contributor

E2E results are ready!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core area: core, team members only embed area: embed, widget, react embed enterprise area: enterprise, audit log, organisation, SAML, SSO ready-for-e2e
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants