Polyfill of promises does not work.

[antoncl]

It would appear that that the 2.x version of less relies on Promises, and from glancing at the code, I can see that an attempt to implement a polyfill of promises has been made, probably in order to be compatible with platforms where promises aren't available yet.

Unfortunately that implementation seems defective. Specifically in line 680 a resolve is called without any parameters. This invocation causes the polyfill (in lines, 9126-9140) to ignore the invocation of 'then'; specifically the 'then' part of the PageLoadFinished handler is never invoked.

In short, the browser based functionality will only work on browsers which has inbuilt support for promises.

[lukeapage]

@ForbesLindesay do you have an opinion on this?
We use "promise" - https://github.com/then/promise which is widely used. Though we might be using it badly somewhere by not providing a resolved value (it seems weird that should be required).

In terms of less.js not working - the browser verison in 2.1 doesn't use promises for normal operation, so it shouldnt be effected. You can see this because we run the browser version on ie8-11.

[ForbesLindesay]

The promise pollyfilled is pretty thoroughly tested and shouldn't create this problem. Can you give me a specific example of a browser it fails in so we can reproduce the error and fix it.

[antoncl]

@ForbesLindesay based on your input about less browser edition working inside ie8-11 I went back in the code again, and and can confirm:

The issue as originally reported by me, can be closed. I was mistaken, the Promise polyfill does indeed work, at least on all the platforms I've been testing on, and on my platform as well.

Naturally I apologize for opening an issue erroneously, sorry, my bad.

But.. what I found is a little interesting none the less (no pun intended).
Back in the old days, the release schedule for the Rhino edition was lagging behind the node based release. In response we grabbed a DOM implementation and hooked it into Rhino, and then ran less - server side - in a simulated browser.

And here is why what we did is relevant: "Rhino does not have an inbuilt setTimeout function!", The polyfill for Promise requires setTimeout. Unfortunately we had a bug in our implementation, specifically the Promise polyfill needs SetTimeout to work as specified for an interval of '0', which our implementation didn't handle correctly.

This caused the Promise system to fail handling Resolves', and thus Less, silently completed without producing any output or error messages.

Which means that if and when the Rhino edition goes live again, it will need a polyfill for setTimeout.

At least I learned a little from this experience, Once again, sorry for the report.

[ForbesLindesay]

Are you aware of any workaround that can be performed to get some kind of setTimeout/setImmediate/process.nextTick like behaviour in Rhino? If so you could suggest it at https://github.com/kriskowal/asap

If that gets merged, it would then be supported by Promise.

[antoncl]

@ForbesLindesay I've been thinking about that, and yes, I think it would be possible. Since I first started looking into this I've been looking at a couple of Promise polyfills, and the one used by less has a special pattern to it.

The milliseconds parameter is always '0' (zero)

Which means that an implementation of something along these lines should work for less:

setTimeout = function(func, millis) { if(millis === 0) { func(); } };

That was the special gotcha, in our implementation, none of us had thought about the possibility of the milliseconds parameter being zero, and what the semantics of that would be.

When running under Rhino there is no reason to be concerned about task scheduling within the executing thread, so in case of a zero wait period, it will be safe to immediately resume operations.

By the way, about the Promises not being used in the browser version of 2.1. Maybe we're using the wrong edition of less, but the one we're using relies heavily on Promises - at least as far as I can tell. We're using the one provided in the 'dist' folder.

`<link rel="stylesheet/less" type="text/css" href="styles.less" />
<script>
    less = {
        env: "development",
        async: false,
        fileAsync: false,
        poll: 1000,
        functions: {},
        dumpLineNumbers: "comments",
        relativeUrls: false,
        rootpath: "",
        logLevel: 3,
        errorReporting: "console"
    };
</script>
<script src="../julia/js/packages/less.js/dist/less.js" type="text/javascript"></script>`

Ever since the 2.x release I haven't been able to find a specific browser edition, but the one provided in 'dist' does the job (if setTimeout behaves as it should).

[ForbesLindesay]

setTimeout with 0 as the time behaves as setTimeout with about 4 or 15 depending on the browser you're using, so it does introduce a delay. The point is not about threading, but rather turns of the event loop. Consider the following code.

// this just creates a promise that is already resolved
var resolved = Promise.resolve(null);

console.log('A');
resolved.then(function () {
  console.log('C');
});
console.log('B');

The promises spec requires that the above example always log A, B, C rather than A, C, B. This must occur even if the promise is already resolved, otherwise it creates inconsistency in the code (see: Unleashing Zalgo).