Closed Bug 1064279 Opened 5 years ago Closed 5 years ago

[meta] Use the Flux pattern for more parts of Loop, and improve the flux style

Categories

(Hello (Loop) :: Client, defect, P5)

defect

Tracking

(Not tracked)

RESOLVED WORKSFORME
Blocking Flags:
backlog tech-debt

People

(Reporter: NiKo, Unassigned)

References

Details

(Whiteboard: [tech-debt])

The more the UI related code for Loop grows, the more painful to maintain it gets: 

- We're passing component dependencies around through props
- We listen to events from componentDidMount() and unregister from componentWillUnmount()
- We often rely on globals from within these component hook methods (navigator.mozLoop, window, document…)

This creates code hard to follow, grasp, track and debug. We should start investigating the Flux pattern[0] which has proven working great combined with React.

Basically the idea is to extract/decouple the app business logic from UI component code, so we'd just use a single app event handler capable of notifying any subscribing component from data state changes.

After discussing with :mikedeboer and :paolo, it seems this would be ideal for handling contacts API mutations, and more generally for anything mozLoop related.

[0]: http://facebook.github.io/react/docs/flux-overview.html
This change will probably be very large and may introduce a fair amount of risk considering that we will need to uplift this to Aurora.

How bad would it be to use the current pattern and only do flux on mozilla-central?
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #1)
> How bad would it be to use the current pattern and only do flux on
> mozilla-central?

That's definitely the plan, yes :)
Whiteboard: [tech-debt]
backlog: --- → Fx36+
Summary: Investigate using the Flux pattern for Loop → [meta] Use the Flux pattern for more parts of Loop
Depends on: 1076754
Depends on: 1079202
Summary: [meta] Use the Flux pattern for more parts of Loop → [meta] Use the Flux pattern for more parts of Loop, and improve the flux style
Depends on: 1088672
Depends on: 1088673
backlog: Fx36+ → Fx37+
Priority: -- → P1
Severity: normal → major
Priority: P1 → P2
backlog: Fx37+ → -
Moved to blocking-loop "-" because this is a meta and the dependent bugs have been bucketed.
didn't get rid of as there's still some break-down of actions that could be added from comments in bug - but biggest ones broken out.
Severity: major → normal
backlog: - → tech-debt
Priority: P2 → P5
We've now got the main currently supported code running in the flux style. There's still some panel code that isn't, but currently we're not going to rewrite that as the cost/benefit doesn't match up at the moment and we'll consider it if it needs extensive work in the future.

The other improvements we should be specific about and file/handle as we go, hence I'm marking this bug as WFM.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.