andrewplummer / Sugar

A Javascript library for working with native objects.
https://sugarjs.com/
MIT License
4.53k stars 306 forks source link

Can't use versions after 1.4.1 with Qt QML #536

Closed bgr closed 8 years ago

bgr commented 8 years ago

I've been using Sugar with Qt's QML for a long time, and it's been very useful. However, I've noticed that upgrading to a newer version won't be possible out-of-the-box, due to the following change:

// The global context
var globalContext = typeof global !== 'undefined' ? global : window;

which raises ReferenceError: window is not defined, since there is no window in QML's JavaScript. I can fix it manually but I just wanted to raise awareness for another platform where Sugar's been used extensively, so that it can be taken into consideration for future changes.

bgr commented 8 years ago

I've tried patching this manually but couldn't succeed, I'll describe my attempts here.

The obvious first step was to change window to this, since that's how it was in the version that works. However, that triggers sugar_patched.js:74: TypeError: Cannot assign to read-only property "Sugar" (offending line is globalContext.Sugar = Sugar;).

Then I tried juggling around, making window an empty object, copying over properties instead of re-assigning etc. but most I could do is get rid of the errors but have Sugar functionality inaccessible (e.g. no create function on Date).

If you get around to doing anything about this, you might find this QML issue relevant. Also, not specific to this issue, but in actual use we prepend the QML-specific .pragma library (docs) at the top of the file as an optimization - just something to be aware of, in case it changes how the code behaves.

bgr commented 8 years ago

Might be relevant: https://github.com/moment/moment/issues/2031

andrewplummer commented 8 years ago

OK, looking into it now... although this was asked in the moment thread, do you have a simple command line workflow to test the environment out? Ideally one that doesn't need me to install 12GB on my machine?

bgr commented 8 years ago

I'm not sure how up-to-date the distro packages are, I've always installed by downloading one of the "offline" Qt installations, that should be pretty straightforward (should work out of the box, no dependencies AFAIK), and significantly less than 12GB :stuck_out_tongue_closed_eyes:.

Then you can create a qml file, e.g. Test.qml:

import QtQuick 2.5
import "sugar.js" as Sugar

Item {
    Component.onCompleted: {
        var r = Date.range(Date.create("26 May 22:00"), Date.create("27 May 04:00"));
        console.log(r.every("30 minutes"));
    }
}

in the same directory where you have a "sugar.js" file, and try running:

qmlscene Test.qml

(qmlscene binary can be found in e.g. .../Qt/Qt5.5.1/5.5/<compiler>/bin)

andrewplummer commented 8 years ago

OK, managed to get up and running and hacked around to get Sugar loaded but it takes a bit of work.

First, I would like to know if there is any way whatsoever to reference the global context in QML. This is the optimal solution as Sugar goes through this when it accesses natives. The main reason for this right now is essentially Sinon, which hijacks the global "Date" object for testing purposes, and I want to support this (plus other similar unavoidable global hijacks). That said, I did get the script up and running by holding onto native objects by name as a fallback, which may also work.

Another reason for wanting the global context is to assign the Sugar value to it, however I understand that QML has a different module import system. One possible solution to this is to create a "QML" build, which simply exports a "Sugar" token like this:

var Sugar = (function () {
  // Main code + init stuff
}).call(this);

This way it can be used as Sugar.Sugar when requiring. It might even be OK to update the current development build with this (right now it's just the closure) as long as it wouldn't have any negative side effects. One question: would potential QML users need a minified build or is the non-minified source code enough?

At any rate, If you can help me find out any way to reference the global, I can have a look at providing support for this.

bgr commented 8 years ago

I'm not that familiar with JavaScript (or QML's JavaScript) internals and terminology, but I guess this gives you the as-global-as-it-gets object, with the caveat that it's read-only, as far as I saw. It seems like it's the global you want - I've tried importing just in a nested child QML component and that makes Sugar functionality available in the parent (which didn't have the import), and vice versa. Note that the example QML that I gave is valid code which works fine with 1.4.1 - Date has create and range, so if a fallback can be elegantly plugged in to make it behave as it did back then, that's perfectly fine by me.

As far as assigning Sugar to global goes, in QML everything that's in the js file is available on the alias you give when importing, so import "../lib/my_file.js" as AnyName will expose all members in my_file.js to be accessed via AnyName.member, if that makes any difference for what you need to do.

I've been using minified version for the last 2 years but only because that's what "Download" button pointed to and I never had to go into source until yesterday. I'd assume it's irrelevant for QML users since it'll be bundled with the app which is huge anyway.

andrewplummer commented 8 years ago

It seems like it's the global you want

It's unfortunately not, as it doesn't have any of the built-in classes on it, for example this.Date, which is what I need...

andrewplummer commented 8 years ago

So, if we assume that the global reference doesn't exist (which it doesn't seem to), and also given your last 2 points, I think the best solution is to update the code to use built-ins by name Date etc, when a reference to the global scope isn't available, then have a qml gulp task that essentially creates a non-minified build with a single var Sugar = ... token output that can then be accessed as Sugar.Sugar. What do you think?

bgr commented 8 years ago

That sounds good, however just to be sure - I never had the need to access anything via Sugar. (that as Sugar in import statement is just because QML requires an alias to be there, it remains unused), I hope that what you're suggesting doesn't mean one would now have to type Sugar.Date.create(...), since that would suck.

andrewplummer commented 8 years ago

OK sorry it seems I was wrong above, the this object in fact is a global reference (I was testing the onCompleted block previously, which appears to be different), however you're right that it's read only, so I can't expose the Sugar global that way.

however just to be sure - I never had the need to access anything via Sugar.

Sorry, I think most of the above statements are confusing because of something I should have clarified earlier. I've been talking about 2.0.0, which is about to be released and functions differently in that it exposes a single Sugar global object. You will have to then call Sugar.extend to extend the natives yourself. Think of this as a one-time init.

The problem you were having previously was because 1.4.1 asks for window, which doesn't exist in QML. 2.0.0 actually fixes this by referring to this instead, (which as mentioned above is in fact the read-only global object), however fails when trying to write to the global. This could be wrapped in a try/catch block so that line doesn't error out, however in order to use 2.0.0 properly you will need to access the Sugar object, which can't be exported to the global since it's read only. Instead, from what I've found, a single var Sugar = will have to be wrapped around the file to follow QML conventions. Then you will have to access that object with Sugar.Sugar. The export keyword could be anything... for example it could be Sugar.foo, however I can't see any way getting around needing something as there doesn't seem to be an equivalent in QML to module.exports = in commonjs, so Sugar.Sugar probably makes the most sense.

I hope that what you're suggesting doesn't mean one would now have to type Sugar.Date.create(...)

Well in fact that's exactly how accessing the global would work, except it would require one more Sugar, so it would in fact be Sugar.Sugar.Date.create. Of course you could easily alias with Sugar = Sugar.Sugar. But don't worry, I know you're more interested in the native extension anyway, so basically all this means is that you will have to call Sugar.Sugar.extend() once, then you will be able to access Date.create as usual. I've already tested this out and it does seem to work.

So basically what I need to do is wrap the global write in a try/catch block, and also write a QML specific wrapper to export the global, which it seems to make sense to me should be a gulp task.

bgr commented 8 years ago

That sounds great. Thanks for finding the solution this quickly!

While you're messing with it, make sure to check if adding .pragma library at the top of the sugar.js file breaks anything. It can be left out from the build, of course.

I wasn't taking 2.0.0 into consideration for upgrading, as I didn't know what shape it's in. Do you suggest upgrading straight to 2.0.0, 1.5.0 or something else? As long as it's considered tested and stable, I'm fine with any (actually all this was summoned by a bug that I encountered in the version I'm currently using, which seems fixed in the meantime).

andrewplummer commented 8 years ago

So I added .pragma library to the top, and it didn't seem to have any side effects, but I'm not 100% sure what it's doing, so I'm not sure what to expect. Seems OK though.

2.0.0 is very close to release (I've said this before on other issues, but I actually mean it this time... talking weeks here), so I suggest waiting if possible. To recap about what I'm doing, essentially what was happening before was this:

(function () {
  // Get the global context
  var context = typeof global !== 'undefined' ? global : this;
  var Sugar = function () { // lots of stuff here... };
  context['Sugar'] = Sugar;
})();

I am going to update to do this:

(function () {
  var context = typeof global !== 'undefined' ? global : this;
  var Sugar = function () { // lots of stuff here... };
  try {
    context['Sugar'] = Sugar;
  } catch(e) {
    // Global may be read-only
  }
})();

Then add a QML task that will wrap the above and output this:

.pragma library
var Sugar = (function () {
  var context = typeof global !== 'undefined' ? global : this;
  var Sugar = function () { // lots of stuff here... };
  try {
    context['Sugar'] = Sugar;
  } catch(e) {
    // Global may be read-only
  }
  return Sugar;
})();

This seems to do the trick. Sound good?

andrewplummer commented 8 years ago

OK that was relatively straightforward. Try pulling the qml branch and running gulp build:qml (probably will have to do npm install). Will output sugar.js by default. Works for me with the following code:

import QtQuick 2.5
import "sugar.js" as Sugar

Item {
    Component.onCompleted: {
        Sugar.Sugar.extend();
        var r = Date.range(Date.create("26 May 22:00"), Date.create("27 May 04:00"));
        console.log(r.every("30 minutes"));
    }
}
bgr commented 8 years ago

Sounds great! I will try it out later on our QML project and report back.

bgr commented 8 years ago

Seems to be working great! (well, almost - I've encountered one error so far, I've reported it separately in #541).