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

Refactor contingencyFlow zoid component to support JS SDK Reloads #51

Merged
merged 1 commit into from
Sep 14, 2020

Conversation

gregjopa
Copy link
Contributor

This PR fixes the "listener already exists" error when reloading the JS SDK when using hosted-fields (originally reported here: paypal/paypal-sdk-client#44).

Screen Shot 2020-09-11 at 9 44 28 PM

The error is fixed by creating the zoid component in the setupHandler instead so it can be cleaned up in the destroy() hook to play nice with the setup/destroy workflow of the sdk-client.

Additional Info

This PR refactors the zoid component to use the same memoize getter approach that the Buttons component uses. This makes it easy to reference the component in multiple places while only creating it once.

This PR breaks up the contingency flow component into two files. I made this change to make it easier to unit test and to follow the zoid folder pattern used in other repos. I learned that the testdouble.js library considers partial mocks an anti-pattern so having two files avoids the need to partially mock the subject.

@@ -257,6 +59,6 @@ function start(url : string) : ZalgoPromise<Object> {
}

export default {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that this export default approach is still needed for the unit tests with how testdouble.js works.

@bluepnume
Copy link
Contributor

Looks great! At some point we should upgrade card-components to use this 3ds component instead: https://github.com/paypal/paypal-common-components/blob/master/src/three-domain-secure/component.jsx -- for now though, this looks like it should work perfectly. Thanks!

@gregjopa
Copy link
Contributor Author

At some point we should upgrade card-components to use this 3ds component instead: https://github.com/paypal/paypal-common-components/blob/master/src/three-domain-secure/component.jsx

Ahh good to know about this shared 3ds component. I'll create a ticket for our backlog to get this migrated.

@gregjopa gregjopa merged commit 57b03f2 into master Sep 14, 2020
@gregjopa gregjopa deleted the DTCHKINT-235 branch September 14, 2020 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants