foray1010 / node-phantom-promise Goto Github PK

First of all, thanks for node-phantom-promise. I like the notion of returning thenables rather than passing callbacks around as it makes for cleaner code.

Unfortunately, it looks like your wrapper breaks one important thing, which is that setting functions to page.onConsoleMessage, page.onCallback, etc., no longer works.

Assume a PhantomJS page under the variable page, opened using the standard pattern:

phantom.create({path: require('phantomjs').path}).then((engine) => {
  engine.createPage().then((page) => {
    ...
  });
});

I should be able to set certain properties to page directly:

page.onConsoleMessage = (msg, lineno, src) => {
  console.log(msg);
};

And then it should work:

// This should log out 'foo' and 'bar', possibly in the reverse order.
page.evaluate(function() {
  console.log('foo');
  return 'bar';
}).then((result) => console.log(result));

It doesn't, though. The onConsoleMessage function never fires. It does in node-phantom-simple, though, so something with the wrapper is breaking it.

You can get around this, kind of, by using setFn, which still works under the wrapper:

page.setFn('onConsoleMessage', function(msg) { console.log(msg) }).then(() => {
  page.evaluate(function() {
    console.log('foo');
    return 'bar';
  }).then((result) => console.log(result));
});

But this is an insufficient workaround because setFn (in node-phantom-simple, not in your code) actually converts the function to a string and then runs it through eval, meaning that all function context is lost (this makes baby moogle cry, kupo!).

Anyway, it would be awesome if direct assignment of page.onWhatever = function () { ... } worked again. I think it needs to be some sort of exemption in promisifyAll but I wasn't certain enough to put together a pull request.