1172008 - [Contacts][NGA] Add Threads.js to Contacts App

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::Contacts, defect)

Description

[Borja Salguero [:borjasalguero]]

Move all our requests to mozContacts API to a model based on Threads.js

Comment 1

[Borja Salguero [:borjasalguero]]

Attachment: WIP

WIP of Contacts using Threads.js. For installing it you will need:
- Execute "bower install" within "apps/communications/contacts" folder
- Execute "make BUILD_APP_NAME=communications install-gaia" at "/gaia" level

Issues to be fixed:
- "make" should execute "bower install" automatically
- Fix test once the WIP patch will be ready

Wilson, Francisco, could you take a look!? Thanks!

Comment 2

[Wilson Page [:wilsonpage]]

How is startup time?

Comment 3

[Wilson Page [:wilsonpage]]

I'm confused, is this (production) ready to replace master contacts app?

Comment 4

[Borja Salguero [:borjasalguero]]

Hi Wilson! This is part of the job we are doing to migrate Contacts App to the new Gaia architecture. Based on the great job we did with the prototype, we are moving all the things we learnt there to Contacts App.

Take into account that Threads.js is already in 'production' [1], that's why I asked today through IRC if we have any proposal about how every app should use (and maintain) it code based on Threads.js.
If every app is uploading it own version of Threads.js we have no control about the code we are running (actually there is no way of inferring the Threads.js version which is used), so we could have a lot of issues in the very near future (we had the same issue with building blocks in the past :)).

What if we tag Threads.js as 'v.0.0.1' as it is in master? Every app will use this version by now, and you can move forward with bug fixing and improvements on the library. Once we decide that a new version is ready (imagine v.0.0.2) we can ask every app to move their code to the latests version of the Threads.js library, overall if we deprecate a version (of if the new version is not compatible with the former one).

Wdyt?


[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/lib/bridge.js

Comment 5

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8616647 [details] [review]
WIP

I though we were going to use an approach like we did for web components.

Use bower, but commit them to shared, but also if each appilcation want's to have their own fetch/download/renaming or whaterver for any reason, they could have their own build step, but ideally all apps should use same version unless (for very specific reasons) each app requires specific versioning.

Wilson, I think that when we were talking about wc a while ago that was the final solution isnt? You have all wc in shared, but each app is free to define their own bower.json, am I right?

Comment 6

[Wilson Page [:wilsonpage]]

(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #5)
> Wilson, I think that when we were talking about wc a while ago that was the
> final solution isnt? You have all wc in shared, but each app is free to
> define their own bower.json, am I right?

Correct. At the end of the dya it's up to the app owner how much control they want over their app dependencies.

Comment 7

[Wilson Page [:wilsonpage]]

Stamped v0.1.2 https://github.com/gaia-components/threads/releases/tag/v0.1.2

Comment 8

[Wilson Page [:wilsonpage]]

Comment on attachment 8616647 [details] [review]
WIP

Overall, looks good.

I would say that the addListener() stuff seems to be making the simple task of event listening rather laborious. Perhaps try simplifying it and then optimizing if it becomes a bottleneck.

I would suggest broadcasting all mozContacts events to all the Service's Client's by default.

// Service code
navigator.mozContacts.addEventListener('contactchanged', e => service.broadcast('contactchanged', ...);

// Client code
client.on('contactchanged', () => ...);

The only drawbacks:

- You may be binding some mozContacts event listeners that aren't currently being used (memory)
- You will be sending a few more BroadcastChannel.postMessage() than are actually needed.

I see both of the above as fairly minor and unlikely to impact performance as the number of navigator.mozContact event types and Clients are likely to be low.

I'd prefer to see clean/readable code, especially as a first example of threads.js usage. If we really need to support this scenario, perhaps we have designed the threads.js events apis wrong?

Comment 9

[BMO Automation]

Firefox OS is not being worked on