Skip to content

Conversation

ryami333
Copy link
Contributor

@ryami333 ryami333 commented Feb 12, 2019

Testing was getting harder, more complex and more verbose as we added more functionality. Not least because testing business logic in React components (with enzyme et al.) is very clunky to begin with. So, it was decided that all of the business logic could be described by a simple ES6 class (the 'store') rather than a React component, and that React components (the context Providers/Consumers) could simply be gateways/APIs to instances of the 'store'.

Changes:

  • Creates 'store' ('AccordionStore') class. Not a React component, just an ordinary ES6 class.
  • Renames 'FooContainer' to 'FooContext', to indicate that those components are simply context consumers/providers and not business-logic-enforcers.
  • Shift's all the business-logic assertions from AccordionContainer.spec (now called AccordionContext.spec) and rewrites them into AccordionStore.spec.
  • Rewrites tests for AccordionContext to assert it's relationship with AccordionStore.

Implications

It should now be much simpler to maintain business logic rules in RAA, because they can all be described by a single class. Making the API changes necessary for v3.0 will now be trivial. Incidentally, we could even unblock server-side-rendering issues now, by exposing an isItemExpanded method in AccordionStore.

@ryami333 ryami333 changed the base branch from master to next February 12, 2019 10:07
@ryami333 ryami333 force-pushed the chore/abstract-business-logic branch from 058f4be to 0a987b1 Compare February 12, 2019 10:14
@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 87.449% when pulling 0a987b1 on chore/abstract-business-logic into 5f29a1e on next.

@ryami333
Copy link
Contributor Author

No interest and PR is only against the next branch, so merging for cadence. Feedback all still welcome though, even on the closed PR.

@ryami333 ryami333 merged commit 1af54b5 into next Feb 14, 2019
@ryami333 ryami333 deleted the chore/abstract-business-logic branch February 14, 2019 06: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