[shared] l10n libraries are not consumable from AMD environments

NEW
Assigned to

Status

Firefox OS
Gaia
4 years ago
4 years ago

People

(Reporter: jugglinmike, Assigned: jugglinmike)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
Currently, Gaia applications authored to use AMD have to special-case Gaia's shared l10n libraries in order to use them effectively [1][2]. If these libraries were defined using the UMD pattern [3], their usage could be greatly simplified. Note that this modification would have no effect on applications that simply include the libraries as browser globals.

[1] https://github.com/mozilla-b2g/gaia/blob/f0f269123886391106ebb37ce738274c1c91df84/apps/clock/js/l10n.js
[2] https://github.com/mozilla-b2g/gaia/blob/f0f269123886391106ebb37ce738274c1c91df84/apps/email/js/l10n.js
[3] https://github.com/umdjs/umd
(Assignee)

Comment 1

4 years ago
Created attachment 8371026 [details] [review]
Pull request on GitHub.com

Hey James,

Marcus and I discussed a change like this in bug 943104. This works for Clock, but I wanted to check with you before attempting to patch E-mail's build system.

What do you think? Is this a reasonable change to suggest? And is this a fair use of named modules?
Attachment #8371026 - Flags: feedback?(jrburke)
Comment on attachment 8371026 [details] [review]
Pull request on GitHub.com

I would prefer to not use a named module for the UMD modifications in /shared library -- a named module is only needed if those files are loaded by manually written script tags and will be used by the loader. However, the app will have best performance options if it uses the loader to load the module. So I would rather encourage best practice there.

I would also not want to prefix all the dependency names with 'moz'. I think just using 'l10n' is enough to indicate the type of dependency, the 'moz' part does not add much useful information. The idea behind module IDs are that they can be swapped out for another provider, for example.

So I do not think this is a pressing priority -- for me this is just about removing the shim config for those resources. While a nice convenience, I would also defer to the owner of the /shared area if they preferred to not do a UMD change.

Updated

4 years ago
Attachment #8371026 - Flags: feedback?(jrburke)
(Assignee)

Comment 3

4 years ago
Hi Kaze,

I'm working with James to make this library code easier to use from AMD applications (currently Clock and E-mail). Would you be open to using the UMD pattern? You can reference the attached pull request on GitHub for what that would look like specifically (I've incorporated Jame's feedback from comment 2).

Note that this does not require a change to any other application that uses the library.
Flags: needinfo?(kaze)
Hi Mike

I’m not against using UMD for l10n.js and l10n_date /per se/, I understand that it would be technically harmless, but I’m worried to implement a specific pattern for shared libraries: no other shared lib is using this pattern so far, and alameda.js is not even a shared lib.

It looks like three apps are already using AMD: Camera, Clock, Email all rely on alameda.js. The Settings app is considering it as well, see bug 964180.

Please raise this subject on m.d.gaia. If we want to make it easier to use AMD and if we want to rely on alameda for that, then we probably should announce it publicly and propose to move alameda.js to a /shared/js/ directory (and probably in a /vendor/ or /3rd-part/ sub-directory). If alameda becomes a shared resource, it will make a lot of sense to use the UMD pattern in l10n.js, l10n_date.js and possibly other shared modules.

I expect this to be a lively discussion. I can see two objections to this:
 • “we should wait for ES6 modules”
 • “if we start with alameda, why not using jQuery/backbone/whatever?”

FWIW, I’m all in favor of reusing existing 3rd-part libraries rather than re-inventing the wheel, but I think we need an open discussion about that.

Arthur, what do you think?
Flags: needinfo?(kaze) → needinfo?(arthur.chen)
There is some background on why I prefer to not put alameda in shared in bug 958856 comment 7. 

Short summary though: I would like to see the shared resources move to their own repos and have apps pull in just the version they know works for them, vs having changes in them force updated to all apps. This is normally how web apps are developed, and it allows the apps finer grained control over its dependencies. It will also set up allowing the apps to live in separate repos in the far, far future. In the case of alameda, it already has a separate repo to track changes and accept pull requests.
(Assignee)

Comment 6

4 years ago
James: I agree with you completely. In order to accomplish this, I believe the following needs to take place:

- Selection of a client-side dependency manager
- Updates to the build process to enable Gaia applications to specify third-party libraries

Is this correct?
Flags: needinfo?(jrburke)
(In reply to James Burke [:jrburke] from comment #5)
> There is some background on why I prefer to not put alameda in shared in bug
> 958856 comment 7.

So it doesn’t make much sense to modify any library in shared/js to be more almeda-friendly, does it?

(In reply to James Burke [:jrburke] from comment #5)
> Short summary though: I would like to see the shared resources move to their
> own repos and have apps pull in just the version they know works for them,
> vs having changes in them force updated to all apps.

I’d love that, James Lal is another supporter of this approach, but I’m not sure it’s reasonable to count on this: a few months ago, this has been considered and rejected because it would cause a significant extra work. That’s why I’m not opposed to factorize some work in shared/js as a mid-term solution.
:jugglinmike: for "client side dependency manager" I assume you mean a package manager? If so, that is such a tarpit, particularly for gaia developers. 

For me, it is enough that each app owner just documents what version/git commit hash was used for each dependency, which could be done manually in a package.json. If the app developer chose to use a client side of their choosing fine, but the main thing is that outside dependencies can be traced to a version and source.

That also assumes, unlike npm usage, that we encourage developers to commit their dependencies to their app tree. It avoids so much hassle and it avoids the need for extensive build changes.

But it is easy for people to bikeshed this into stop energy. For me, getting third party scripts to separate repos, and doing the commits of dependencies into app directories was the least friction way to get to separate app repos.

And all of that is out of scope for this ticket. End result: I do not see alameda going into shared/js any time soon.

:kaze: modifying shared/js libs for module use is really a separate issue from whether the mechanism for how they are loaded comes the shared/js. UMD-style registration is a fairly common library practice. There are three apps in the gaia repo that would have a slightly better usage experience with it, but they also will not break if these libraries are not changed.
Flags: needinfo?(jrburke)
Shim works well for consuming non-AMD modules. The problem here seems resulted from the dependency between shared/js libraries, and which is rare. As comment 8 suggested, if alameda is not going into shared/js soon, and for the consistency of the shared libraries, I think apps might have to compromise with this now.
Flags: needinfo?(arthur.chen)
You need to log in before you can comment on or make changes to this bug.