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

Request ID should be sometimes undefined #662

Closed
jkjustjoshing opened this issue Aug 16, 2024 · 3 comments · Fixed by #664
Closed

Request ID should be sometimes undefined #662

jkjustjoshing opened this issue Aug 16, 2024 · 3 comments · Fixed by #664
Labels
new Flag for new issues before someone replies released

Comments

@jkjustjoshing
Copy link

The event argument type passed to the onSubmit callback (and probably others, I didn't check) can be undefined when sandbox mode is enabled. However, the type definition says it's always a string.

Can be seen when using React

<Widget 
  onSubmit={(event) => console.log(typeof event.responseId)}
  sandboxMode={true}
/>

event.responseId is undefined when sandbox mode is enabled, but the type definition is string, not string | undefined.

The fix

Update this line to be

  onSubmit?: (event: WithFormId & Partial<WithResponseId>) => void

However, that may indicate responseId is missing, not set to undefined, so you could also update WithResponseId to be { responseId: string | undefined }, but I'm not sure where else that type is used and what the repercussion is.

@github-actions github-actions bot added the new Flag for new issues before someone replies label Aug 16, 2024
@mathio
Copy link
Contributor

mathio commented Aug 19, 2024

Hello @jkjustjoshing

Good catch! From the top of my head I dont see any issues with updating this to { responseId: string | undefined }. I think we can update the type.

@mathio
Copy link
Contributor

mathio commented Aug 29, 2024

Hello again.

After considering changing types, we will be passing a "__sandbox" string value for responseId instead. Changing the type would mean everyone implementing the onSubmit callback would need to check if the responseId is defined, even if they do not care about the sandbox (I assume most customers do not implement sandboxed embeds).

I also noticed the onStarted was not called for sandboxed embeds at all. The upcoming change will also fix this (it will be called with object { formId: "actual-form-id", responseId: "__sandbox" } as param). Docs update coming after the change is released.

mathio added a commit that referenced this issue Aug 30, 2024
mathio added a commit that referenced this issue Aug 30, 2024
mathio added a commit that referenced this issue Aug 30, 2024
@mathio mathio closed this as completed in be89ed2 Sep 2, 2024
typeform-ops-gha pushed a commit that referenced this issue Nov 20, 2024
# [@typeform/embed-v5.2.0](https://github.com/Typeform/embed/compare/@typeform/embed-v5.1.0...@typeform/embed-v5.2.0) (2024-11-20)

### Bug Fixes

* **NOJIRA-123:** Prevent loading live embed multiple times ([#668](#668)) ([ac398e0](ac398e0))

### Features

* **TU-17542:** Update docs on sandbox mode ([#664](#664)) ([be89ed2](be89ed2)), closes [#662](#662)
@typeform-ops-gha
Copy link

🎉 This issue has been resolved in version @typeform/embed-v5.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new Flag for new issues before someone replies released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants