search

gilbarbara / react-joyride

Create guided tours in your apps
https://react-joyride.com/
MIT License
6.62k stars 519 forks source link

Update CallBackProps' origin to be optional #1047

Open sastaachar opened 4 days ago

sastaachar commented 4 days ago

Since origin is only passed if available and can be null, it should be optional.

codesandbox-ci[bot] commented 4 days ago

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

gilbarbara commented 4 days ago

Hey @sastaachar

The origin is set internally for the props the callback receives, so why do you want to change it?

sastaachar commented 4 days ago

Hey @sastaachar

The origin is set internally for the props the callback receives, so why do you want to change it?

Was using it for internal testing and mocking , it was not required in 2.2 but from 2.8 it is , thought its breaks 2.x contract .

Also since its | null thought optional would be correct here ?

Feel free to close this if its not appropriate. :)

gilbarbara commented 4 days ago

Hey!

Yeah, but adding a property doesn't break the contract per se unless you're testing the implementation, which, in this case, might force you to add the origin to your mock.

Also since its | null thought optional would be correct here ?

Not really. Since null is a defined value while undefined, you can't be sure if it was set or there's nothing.

sastaachar commented 4 days ago

yup makes sense , thanks 👍