Hooks refactor

[tbrannam]

Table of Contents

• Description
• Motivation and Context
• How Has This Been Tested?
• Screenshots (if appropriate):
• Types of changes
• Checklist:

Description

Introduce hooks support
Refactor CoreExperiment to use hooks
Update tests.
Fix slow tests

Motivation and Context

Ready support for react 17.x
Add hooks support for React Functional Components

How Has This Been Tested?

Updated tests related to params changes for core experiment.
Added specific tests for hook, useExperiment

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

[moretti]

Hey @tbrannam, looks like an amazing work, I'll have deep look tomorrow.
Having support for hooks is great, I think we'll have to create a major release, since we're no longer supporting [email protected] and react@15, but it should be fine, we can potentially add it to README.

I'm sorry about the late reply (currently drowning in reviews 😅), thank you for submitting this pull request! 🎉

[tbrannam]

It's all good - I think I did everything I wanted to do - just added tests for hooks.

[moretti]

This is really great work! 🎉
I left a couple of suggestions

[tbrannam]

Think I've addressed all the outstanding issues.

One thing that I wonder about revisiting is the need to preregister variants. This is due to the Hook not having any reference to the possible list of variants like the Components would have. I like the hook interface now, but it might work if somehow the variants were passed to the hook as well as an alternative calling pattern.

Or maybe it isn't too bad the way it is?

[tbrannam]

friendly bump

[moretti]

I had another deep look at source code and I think it looks really good.

One way to solve the last problem that you mentioned would be to extend the useExperiment hook, eg. use an object for advance configuration:

useExperiment('experiment_name', { 
  variants: ['A', 'B'],
  defaultVariant: 'A',
  userIdentifier: 'pk_123', 
});

I think we could potentially merge this PR as it is, release a beta version of v3, test it for a few days and see how it goes, what do you think?

[tbrannam]

Sounds good

…

________________________________ From: Paolo Moretti <[email protected]> Sent: Tuesday, September 17, 2019 5:32:42 AM To: marvelapp/react-ab-test <[email protected]> Cc: Todd Brannam <[email protected]>; Mention <[email protected]> Subject: Re: [marvelapp/react-ab-test] Hooks refactor (#10) I had another deep look at source code and I think it looks really good. One way to solve the last problem that you mentioned would be to extend the useExperiment hook, eg. use an object for advance configuration: useExperiment('experiment_name', { variants: ['A', 'B'], defaultVariant: 'A', userIdentifier: 'pk_123', }); I think we could potentially merge this PR as it, release a beta version of v3, test it for a few days and see how it goes, what do you think? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#10?email_source=notifications&email_token=AAATHXN7ENKIOJ3SKPSMJNLQKCP3VA5CNFSM4IUR4FW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD635R7A#issuecomment-532142332>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AAATHXMIRLUYZZ3MUO5EHN3QKCP3VANCNFSM4IUR4FWQ>.

[moretti]

Released in 3.0.0-beta.0 🎉