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

feat(reakit-system): Replace useCompose by useComposeOptions on createHook #493

Merged
merged 1 commit into from
Nov 14, 2019

Conversation

diegohaz
Copy link
Member

@diegohaz diegohaz commented Nov 14, 2019

useCompose kind of breaks the encapsulated composition approach of createHook. This PR replaces it by useComposeOptions, which will be called after useProps and return options instead of calling the composed hooks directly.

Also, it changes the order that composed useOptions are called. Before, they were being called after the current hook useProps, which didn't compose so well as it wouldn't have access to options modified by the composed hooks. Now composed useOptions are called right after the current hook useOptions, but before useProps. The current hook useProps now will receive the options after they get proccessed by all composed hooks. It's necessary so the new Id module proposed on #492 will be able to pass a default id or baseId option to other hooks composing it.

Does this PR introduce a breaking change?

There's no breaking changes on the main package (reakit), but reakit-system's createHook won't accept useCompose anymore.

  • createHook's useCompose has been replaced by useComposeOptions.

    Before:

    const useHook = createHook({
      useCompose(options, htmlProps) {
        return useComposedHook({ ...options, newOption: true }, htmlProps);
      }
    });

    After:

    const useHook = createHook({
      useComposeOptions(options) {
        return { ...options, newOption: true };
      }
    });

@codecov
Copy link

codecov bot commented Nov 14, 2019

Codecov Report

Merging #493 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #493      +/-   ##
==========================================
- Coverage   95.43%   95.42%   -0.01%     
==========================================
  Files         100      100              
  Lines        1490     1487       -3     
  Branches      472      472              
==========================================
- Hits         1422     1419       -3     
  Misses         68       68
Impacted Files Coverage Δ
packages/reakit/src/Menu/Menu.tsx 100% <ø> (ø) ⬆️
packages/reakit/src/Form/FormInput.ts 100% <100%> (ø) ⬆️
packages/reakit/src/Form/FormCheckbox.ts 92.85% <100%> (ø) ⬆️
packages/reakit/src/Menu/MenuDisclosure.ts 98% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d87e99...ba61f5d. Read the comment docs.

@ariakit-bot
Copy link

Deploy preview for reakit ready!

Built with commit ba61f5d

https://deploy-preview-493--reakit.netlify.com

@diegohaz diegohaz merged commit 50fd7df into master Nov 14, 2019
@diegohaz diegohaz deleted the feat/reakit-system-use-compose-options branch November 14, 2019 16:02
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.

2 participants