844426 - [email] Limit JS syntax to legal ES5 syntax

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Description

[Andrew Sutherland [:asuth] (he/him)]

In order to process the e-mail app through more aggressive minifiers like Google's closure compiler or uglify/uglify2, we need to remove our non-ES5-happy uses of 'const', 'let', and destructuring assignment.

Vivien did a pass at this in his branch to add worker support, nominally tracked in bug 814257.  I'm porting these changes from the branch against the gaia repo to be against the gaia-email-libs-and-more and the repositories it reuses using the fantastic "meld" merge/diff tool.  Changes to semantics are not being taken so the worker-related changes don't get conflated.

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

Attachment: https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/130

This pull request is currently in progress.  So far I think I've ported all the required changes from the mailapi/ dir.  Will do jsas/wbxml shortly and then any files outside of mailapi/

Comment 2

[James Burke [:jrburke]]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/8306

Pointer to Github pull-request

Comment 3

[James Burke [:jrburke]]

Comment on attachment 718069 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/8306

Just removes the non-ES5 things from the front end code. The back end changes for the scripts in js/ext will be part of a separate change being done by asuth.

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 718069 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/8306

Landed on gaia/master:
https://github.com/mozilla-b2g/gaia/pull/8306

Leaving open for the back-end changes.

Comment 5

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 717480 [details] [review]
https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/130

These changes carry rs=squib, contingent upon unit tests passing.  All unit tests pass that already passed; wbxml, activesync, and gaia-email-libs-and-more.

A smoke-test on an unagi device of both ActiveSync and IMAP runs happily.

The main changes required in addition to the changes ported from Vivien's gaia branch:
- There were changes in jsas that had not been taken into GELAM, so test failures mistakenly cropped up that were not due to regressions in the port.
- Some of the yield stuff for ActiveSync had been commented out rather than translated.  I finished the translation.
- Some of the (var iter) declarations occurred in a scary nested fashion.  In practice, these were not a big problem because 'iter' was an extremely temporary variable, but since it's the type of thing that can draw the eye and result in concern, I fixed those too.

I believe this completes the ES5-syntax-only changes because when I did "npm install uglify" and ran the uglify command line against all of our aggregations in gaia, they all produced output that looked minified and did not explode.

When I changed the gaia-email-opt.build.js file to use 'uglify' instead of 'none' and altered onBuildWrite to return the modified contents, nothing actually changed.  I may have been being too naive; I didn't look into it all that deeply.

I am going to merge this now and mark fixed very shortly...  REOPEN if it seems like I didn't get everything.

Comment 6

[Andrew Sutherland [:asuth] (he/him)]

landed on gaia-email-libs-and-more/master:
https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/130

landed on gaia/master:
https://github.com/mozilla-b2g/gaia/pull/8399


note: I left the mozilla-b2g/gaia-email-libs-and-more branch open in case it turns out we need some more changes to let uglify do its thing.

Comment 7

[James Burke [:jrburke]]

For gaia-email-opt.build.js, it is used by copy-to-gaia.js, and copy-to-gaia was doing some manual concatenation in and part due to the non-ES5 code translations needed for build analysis. Once this change lands, then the gaia-email-opt.build.js and copy-to-gaia.js duality could be reduced, and the optimize: 'uglify' would then be available.

Comment 8

[James Burke [:jrburke]]

I'm having trouble getting the latest master code up to date. It has to do with the activesync submodule. This could very well be user error, but when trying to update it, I get:

Unable to checkout '55621b4e5d3a4ba01e429fb7c12eccc7cd5b79e9' in submodule path 'deps/activesync'

Looking here:
https://github.com/mozilla-b2g/jsas/commits/master

I do not see that commit hash, does that project need to have a push?

Comment 9

[Andrew Sutherland [:asuth] (he/him)]

Thanks for the heads-up James.  I believe I've fixed the problem now.  A recursive checkout of GELAM now works fine.  I should probably write up a formal checklist for myself when the more footgun aspects of git submodules com einto play...

Comment 10

[Andrew Sutherland [:asuth] (he/him)]

I was just cleaning up some of my other repos, and it did seem like a "git submodule sync" may in fact be required in gaia-email-libs-and-more, or from within gaia-email-libs-and-more/deps/activesync.

Comment 11

[Julien Wajsberg [:julienw] (PTO -> Mar 16)]

Was landed in v1.0.1 as part of Bug 851124 big uplift.
Will land (again) in v1-train tomorrow at the latest too.

Comment 12

[Julien Wajsberg [:julienw] (PTO -> Mar 16)]

Landed in v1-train as part of Bug 851124 big uplift.