After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 778425 - Make gnome-shell ready for new GJS
Make gnome-shell ready for new GJS
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
3.23.x
Other Linux
: Normal normal
: ---
Assigned To: Philip Chimento
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2017-02-10 02:08 UTC by Philip Chimento
Modified: 2017-02-11 01:01 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
main: Destroy GjsContext before exit (1.25 KB, patch)
2017-02-10 02:14 UTC, Philip Chimento
committed Details | Review
ibusManager: Use const correctly (1.06 KB, patch)
2017-02-10 02:14 UTC, Philip Chimento
committed Details | Review
js: Avoid double declarations with let (3.07 KB, patch)
2017-02-10 02:14 UTC, Philip Chimento
none Details | Review
js: Avoid double declarations with let (3.02 KB, patch)
2017-02-10 21:17 UTC, Philip Chimento
committed Details | Review

Description Philip Chimento 2017-02-10 02:08:34 UTC
I've been testing gnome-shell with the wip/ptomato/mozjs38 branch of GJS and a few changes are needed. The main issue is that the following code is a syntax error in ES6, but was previously quietly accepted by GJS:

    let a = 'something';
    let a = 'other thing';

The second line must be changed to "a = 'other thing';".
Comment 1 Philip Chimento 2017-02-10 02:14:14 UTC
Created attachment 345395 [details] [review]
main: Destroy GjsContext before exit

Here are a few places where I missed it before: the GjsContext must be
unrefed before exiting, or it will crash on exit.
Comment 2 Philip Chimento 2017-02-10 02:14:19 UTC
Created attachment 345396 [details] [review]
ibusManager: Use const correctly

Per ES6, a variable declared const should only be valid inside its lexical
scope. Previously, GJS would accept this code, but that will change in the
SpiderMonkey JS engine in the next release of GJS.
Comment 3 Philip Chimento 2017-02-10 02:14:28 UTC
Created attachment 345397 [details] [review]
js: Avoid double declarations with let

The following code is a syntax error in ES6:

    let a = 'something';
    let a = 'other thing';

Previously GJS would silently accept this code, but in the next release the
SpiderMonkey JS engine will be more ES6-compliant.
Comment 4 Florian Müllner 2017-02-10 10:36:28 UTC
Review of attachment 345395 [details] [review]:

LGTM
Comment 5 Florian Müllner 2017-02-10 10:36:34 UTC
Review of attachment 345396 [details] [review]:

Dto.
Comment 6 Florian Müllner 2017-02-10 10:36:40 UTC
Review of attachment 345397 [details] [review]:

::: js/ui/endSessionDialog.js
@@ +458,3 @@
         _setLabelText(this._subjectLabel, subject);
 
+        dialogContent = DialogContent[this._type];

Removing the 'let' is correct of course, but the whole reassignment looks bogus to me - the variable is already set to DialogContent[this._type] and not modified.
Comment 7 Philip Chimento 2017-02-10 21:17:02 UTC
Created attachment 345486 [details] [review]
js: Avoid double declarations with let

The following code is a syntax error in ES6:

    let a = 'something';
    let a = 'other thing';

Previously GJS would silently accept this code, but in the next release the
SpiderMonkey JS engine will be more ES6-compliant.
Comment 8 Philip Chimento 2017-02-10 21:20:24 UTC
Attachment 345395 [details] pushed as d017e67 - main: Destroy GjsContext before exit
Attachment 345396 [details] pushed as f7752ac - ibusManager: Use const correctly
Comment 9 Florian Müllner 2017-02-10 21:58:03 UTC
Review of attachment 345486 [details] [review]:

LGTM
Comment 10 Philip Chimento 2017-02-10 22:39:09 UTC
Attachment 345486 [details] pushed as aefd61c - js: Avoid double declarations with let