Closed Bug 1094137 Opened 10 years ago Closed 10 years ago

Create a common, shared Flux Store creator for Loop

Categories

(Hello (Loop) :: Client, defect)

defect
Not set
normal
Points:
2

Tracking

(firefox35 fixed, firefox36 fixed)

RESOLVED FIXED
mozilla36
Iteration:
36.3
Tracking Status
firefox35 --- fixed
firefox36 --- fixed

People

(Reporter: NiKo, Unassigned)

References

Details

(Whiteboard: [tech-debt])

Attachments

(1 file, 4 obsolete files)

We need a common base Flux store creator, so we could avoid reimplementing & duplicating the store shared API by hand all the time.

Example target usage:

var StandaloneAppStore = loop.store.createStore({
  registeredActions: [
    "getWindowData"
  ],
  getWindowData: function(actionData) {
    // …
  }
});
Attached patch Shared Flux store creator. (obsolete) — Splinter Review
This patch brings a loop.store.createStore function to ease creating Flux storesi sharing a common stadard API.

RoomStore and ActiveRoomStore have been migrated to use this new facility, other stores should follow in a possible part 2/follow up bug.

Note that ConversationStore is harder to migrate because it extends Backbone.Model.
Attachment #8517699 - Flags: review?(standard8)
Removed duplicate dispatcher passed as options to stores.
Attachment #8517699 - Attachment is obsolete: true
Attachment #8517699 - Flags: review?(standard8)
Attachment #8517703 - Flags: review?(standard8)
Attached patch Shared Flux store creator, v3. (obsolete) — Splinter Review
Added BaseStore#dispatchAction to ease dispatching right from a store instance.
Attachment #8517703 - Attachment is obsolete: true
Attachment #8517703 - Flags: review?(standard8)
Attachment #8517952 - Flags: review?(standard8)
Comment on attachment 8517952 [details] [diff] [review]
Shared Flux store creator, v3.

Review of attachment 8517952 [details] [diff] [review]:
-----------------------------------------------------------------

This is good, I like the consistency it introduces and the functions it has. I suspect there's some small bitrot, and there's a few comments to be addressed, but I think now is a good time to land this.

For the checkin comment, I'd go with something like "Bug 1094137 - Create a comment shared store creator for Loop."

::: browser/components/loop/content/shared/js/activeRoomStore.js
@@ +19,5 @@
>     *
>     * @extends {Backbone.Events}
>     *
>     * @param {Object}          options - Options object
>     * @param {loop.Dispatcher} options.dispatch - The dispatcher for dispatching

jsdoc needs updating - dispatcher is no longer part of options.

::: browser/components/loop/content/shared/js/roomStore.js
@@ +54,5 @@
>     * - {ActiveRoomStore} activeRoomStore  An optional substore for active room
>     *                                      state.
>     *
>     * @extends {Backbone.Events}
>     * @param {Object} options Options object.

jsdoc needs updating - dispatcher is no longer part of options.

::: browser/components/loop/content/shared/js/store.js
@@ +6,5 @@
> +
> +var loop = loop || {};
> +loop.store = loop.store || {};
> +
> +(function() {

I kinda prefer this format:

loop.store.createStore = (function() {

as it gives you an idea of what's going to be returned for the function.

@@ +35,5 @@
> +     * Returns current store state. You can request a given state property by
> +     * providing the `key` argument.
> +     *
> +     * @param  {String|undefined} key An optional state property name.
> +     * @return {Object}

This might not be true if a key is specified - it could be string/number etc.
Attachment #8517952 - Flags: review?(standard8) → review+
Assignee: nobody → nperriault
Rebased patch on top of latest fx-team; as this involved revamping part of the API (eg. getInitialStoreState), requesting a new review here.
Attachment #8517952 - Attachment is obsolete: true
Attachment #8524695 - Flags: review?(standard8)
Dead code & obsolete jsdocs removal.
Attachment #8524695 - Attachment is obsolete: true
Attachment #8524695 - Flags: review?(standard8)
Attachment #8524838 - Flags: review?(standard8)
Comment on attachment 8524838 [details] [diff] [review]
Create a shared store creator for Loop. Patch v4.1.

Review of attachment 8524838 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.
Attachment #8524838 - Flags: review?(standard8) → review+
https://hg.mozilla.org/integration/fx-team/rev/60a876878c55
Iteration: --- → 36.3
Target Milestone: --- → mozilla36
Whiteboard: [tech-debt]
Comment on attachment 8524838 [details] [diff] [review]
Create a shared store creator for Loop. Patch v4.1.

Approval Request Comment
[Feature/regressing bug #]: N/A
Required for Rooms support in 35
[Describe test coverage new/current, TBPL]: on m-c, rooms is now enabled on Nightly

[String/UUID change made/needed]: none
Attachment #8524838 - Flags: approval-mozilla-aurora?
Attachment #8524838 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: qe-verify-
Flags: in-moztrap-
You need to log in before you can comment on or make changes to this bug.