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(screeb-sdk-react): fix surveyStart helper typing #34

Merged
merged 1 commit into from
Oct 2, 2024

Conversation

vincsb
Copy link
Contributor

@vincsb vincsb commented Oct 2, 2024

👋,

I'd like to suggest a change on the surveyStart type definition in @screeb/sdk-react package.

Since this recent commit e2f4b6c a new param has been added to the function signature (language) and it seems optional like the hooks param.

It seems to have an inconsistency between the implementation and the type definition in packages/screeb-sdk-react/src/types.ts.

In this type, hooks and language was added as mandatories.

I can be wrong but this looks like a breaking change when using the sdk because 2 news params are now mandatories and we can't upgrade until we use it (even the hooks).

Can you consider this and having a look?

Thanks for your help 🙏

@cley44 cley44 merged commit 169dc4c into ScreebApp:master Oct 2, 2024
1 check passed
@cley44
Copy link
Contributor

cley44 commented Oct 2, 2024

👋,

It is indeed an oversight on our part. Thanks for the PR !

@vincsb
Copy link
Contributor Author

vincsb commented Oct 2, 2024

Thanks for the quick answer!

@vincsb vincsb deleted the fix-sdk-react-surveystart-type branch October 2, 2024 12:55
@cley44
Copy link
Contributor

cley44 commented Oct 2, 2024

I just published the version 0.1.12 with the fix

@vincsb
Copy link
Contributor Author

vincsb commented Oct 3, 2024

Thanks, I'm waiting for the version on npm registry (@screeb/sdk-react@npm:0.1.12: No candidates found for now)

=> https://www.npmjs.com/package/@screeb/sdk-react?activeTab=versions

@MD4
Copy link
Collaborator

MD4 commented Oct 4, 2024

Hi @vincsb 👋
I just published it @screeb/[email protected] :)

@vincsb
Copy link
Contributor Author

vincsb commented Oct 7, 2024

👋 @MD4
Thanks, I was able to upgrade to last version without any regressions now 👍

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