1169361 - Switch to require('./l20n') in webapp-optimize

Status: RESOLVED FIXED

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

Description

[Staś Małolepszy :stas]

The new localization library (l20n.js) is fully async and uses Promises.  The current webapp-optimize code relies on the fact that the file IO is sync on buildtime (and it mocks XHR in a sync manner).  With Promises it won't be possible to rely on the sync IO anymore and we'll need to refactor webapp-optimize to be async.

Comment 1

[Staś Małolepszy :stas]

I have a WIP branch at https://github.com/stasm/gaia/tree/1169361-build-async which is based on my fix for bug 1169309.  The relevant diff is:

https://github.com/stasm/gaia/compare/1169309-separate-concat-pretranslate...stasm:1169361-build-async

It doesn't work.  I don't understand utils.processEvents here and how it relates to my async code:

https://github.com/stasm/gaia/compare/1169309-separate-concat-pretranslate...stasm:1169361-build-async#diff-b8c773ce202d23e4e6718b08f32ecf3cL92

Ricky, could you point me into the right direction?  Does this WIP even make sense to you? :)

Comment 2

[Ricky Chien [:rickychien] (inactive)]

Documentation of utils.processEvents is in here: https://github.com/mozilla-b2g/gaia/blob/4b20609d3f69f08d5f438a8471ccd2b8c4e78917/build/utils-xpc.js#L695-L716

Another document for nsiThread see: https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIThread#processNextEvent()

I'm also not familiar with these XPCOM APIs, I guess utils.processEvents can block current thread until events are done, so please see this use case of build/settings.js https://github.com/mozilla-b2g/gaia/blob/4b20609d3f69f08d5f438a8471ccd2b8c4e78917/build/settings.js#L327-L328.

You have to put utils.processEvents under all your async code to wait until promises are finished. After that, webapp-optimize can still be treated as a sync modules as usual, which means you don't have to wrap app.js, build-app,js with promise style. I don't verify it yet, but I hope we're able to sort this out by this solution.

Comment 3

[Staś Małolepszy :stas]

Thanks for the pointers, Ricky, they are very helpful!  I'll see what I can come up with.

Comment 4

[Staś Małolepszy :stas]

OK, I was able to use utils.processEvents, thanks Ricky!  I also had to change how HTMLOptimizer.win was created:  it used to be a global object available for all HTML files in the given app.  Each file would then initialize win.document and work with it.  Moving to async means we can no longer rely on win being a global, so I had to move it into HTMLOptimizer.process to be local to each file.

I realized I should be able to simplify build's l10n.js a little bit, so I'm still working on this PR.

My ultimate goal is to rewrite build's l10n.js such that it can be required from webapp-optimize.  That's going to be a separate bug though.

Comment 5

[Staś Małolepszy :stas]

(In reply to Staś Małolepszy :stas from comment #4)

> My ultimate goal is to rewrite build's l10n.js such that it can be required
> from webapp-optimize.  That's going to be a separate bug though.

Scratch that.  Let's do this in this bug.  In bug 1175260 I landed shared/js/l20n.js and now I'd like to follow up with the build part.

I was able to remove the old l10n.js completely from webapp-optimize and replace it with l20n.js which can be required.  This means no more clunky mock window object with fake XHR etc.!

Comment 6

[Staś Małolepszy :stas]

Attachment: Pull request

Comment 7

[Staś Małolepszy :stas]

Comment on attachment 8629347 [details] [review]
Pull request

Hi Ricky -- in this PR I removed the mockwin logic from webapp-optimize which was required to make shared/js/l10n.js work in XPC. Instead, I'm using the nex-gen version of the localization library, l20n.js, which can be require()'ed.  The code lives in build/l10n/l20n.js and is the output of babel-transpilation of https://github.com/l20n/l20n.js/tree/v3.x.  Can you take a look at the changes to build/webapp-optimize and let me know if they look good?  l20n.js uses Promises so I needed to work around the asynchronicity with util.processEvents.

Zibi, please review the l20n.js and the multilocale parts.

Comment 8

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8629347 [details] [review]
Pull request

Great! I'm glad to see webapp-optimize.js would become more clear after l20n landed and it also reduces more pain for migrating to node.js (especially in mock win object).

Build part seems good to me!

Comment 9

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8629347 [details] [review]
Pull request

r+

would love to see more readable output, so we'll work with Stas on switching away from webpack to our own babel bundler.

Comment 10

[Staś Małolepszy :stas]

https://github.com/mozilla-b2g/gaia/commit/bdddfe1ebb796e2bc1c048d5c4e0f97f3d06f98b

\o/

Comment 11

[Zibi Braniecki [:zbraniecki][:gandalf]]

Stas, I think I found a bug in the l20n serializer. The resulting JSON file is an array with an object inside. I believe it should be just an object.

Comment 12

[Staś Małolepszy :stas]

Ah, confirming :(  I'll fix that in a follow-up and add a test using l20n-app.  Thanks!

Comment 13

[Staś Małolepszy :stas]

(In reply to Staś Małolepszy :stas from comment #12)
> Ah, confirming :(  I'll fix that in a follow-up and add a test using l20n-app.  Thanks!

Bug 1182406.