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

executeAsync: Promised based execution GH-72 #163

Merged
merged 7 commits into from Jun 6, 2020
Merged

executeAsync: Promised based execution GH-72 #163

merged 7 commits into from Jun 6, 2020

Conversation

stemsmit
Copy link
Contributor

Addresses #72, adds executeAsync to return a promise which resolves and rejects based on handleChange and handleErrored

@stemsmit
Copy link
Contributor Author

stemsmit commented Jan 17, 2020

Worth noting that Promise isn't available in IE. I can add the polyfill if need be.

@Morkowski
Copy link

+1

@johanosventer
Copy link

What's the status of this PR? Looks like a couple of linting fixes only?

Copy link
Collaborator

@hartzis hartzis left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution! I think the async execute is a great idea.

Could you additionally add some brief documentation about the new api to the README?

test/recaptcha.spec.js Outdated Show resolved Hide resolved
}
const instance = ReactTestUtils.renderIntoDocument(React.createElement(WrappingComponent));
let result = instance._internalRef.current.executeAsync();
expect(result).toBeInstanceOf(Promise);
Copy link
Collaborator

@hartzis hartzis Feb 3, 2020

Choose a reason for hiding this comment

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

💭 add an additional expect to test that the resolved value should be the token?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, sorry if I am missing something, how would I get the test value of token to compare against?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hey @stemsmit , looks like there should be a couple different ways via the jest docs.

async/await should work too, but i think i see what you're talking about. We'll need to setup a slightly sophisticated grecaptchaMock...

const WIDGET_ID = "someWidgetId";
const TOKEN = "someToken";
const grecaptchaMock = {
      render(_, { callback }) {
        this.callback = callback;
        return WIDGET_ID;
      },
      execute() { this.callback(TOKEN) },
    };
...
let result = instance._internalRef.current.executeAsync();
const retrievedToken = await result;
expect(retrievedToken).toBe(TOKEN);

Copy link
Contributor Author

@stemsmit stemsmit Feb 4, 2020

Choose a reason for hiding this comment

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

I attempted to use async / await but ran into an issue with regeneratorRuntime rather than make changes to fix that issue I set it up to return the promise with the expect inside a then callback.

@stemsmit
Copy link
Contributor Author

stemsmit commented Feb 4, 2020

Thank you for this contribution! I think the async execute is a great idea.

Could you additionally add some brief documentation about the new api to the README?

Readme changes added.

Copy link
Collaborator

@hartzis hartzis left a comment

Choose a reason for hiding this comment

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

This looks great. Thanks for the PR.

@dozoisch This is backwards compatible and could be released as a minor. Let us know what you think and also thank you for your continued support for this project.

Comment on lines +116 to +144
```jsx
import ReCAPTCHA from "react-google-recaptcha";


const ReCAPTCHAForm = (props) => {
const recaptchaRef = React.useRef();

const onSubmitWithReCAPTCHA = async () => {
const token = await recaptchaRef.current.executeAsync();

// apply to form data
}

return (
<form onSubmit={onSubmitWithReCAPTCHA}>
<ReCAPTCHA
ref={recaptchaRef}
size="invisible"
sitekey="Your client site key"
/>
</form>
)

}

ReactDOM.render(
<ReCAPTCHAForm />,
document.body
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 Awesome example

@etienne-bondot
Copy link
Contributor

When do you think this could be merged ? This is very cool!

@dozoisch
Copy link
Owner

I'll try to merge this, this weekend, sorry for the delay.

@liweiyi88
Copy link

This feature looks pretty useful. May I ask when will you be able to release this feature?

@UseMuse
Copy link

UseMuse commented May 23, 2020

please include this PR in the project already) just do it
@dozoisch How is it going?)

@SDrinkwater
Copy link

@dozoisch Can we help in any way to get this moving along? Looks like it has been sitting in a ready-to-merge state for the last 4 months or so.

@dozoisch dozoisch merged commit 1ec12aa into dozoisch:master Jun 6, 2020
@dozoisch
Copy link
Owner

dozoisch commented Jun 6, 2020

2.1 published

@SDrinkwater
Copy link

Amazing. Thanks!

@stemsmit stemsmit deleted the GH-72_ExecuteAsync branch June 6, 2020 06:02
@stemsmit
Copy link
Contributor Author

stemsmit commented Jun 6, 2020

@hartzis
Copy link
Collaborator

hartzis commented Jun 13, 2020

🎉 Nice.

Also updated the "invisible" example to showcase the new executeAsync.

https://codesandbox.io/s/gifted-cache-10q74jj593?file=/src/index.js

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.

None yet

9 participants