run alasql in nodejs vm

[xlvector]

We found there is some problems about run alasql in nodejs vm, this is mainly because of this issue.

nodejs/node-v0.x-archive#1277

In alasql, there is many line like (x instanceof Array) to check if x is an array, and the better way is Array.isArray(x). Use (x instanceof Array) will cause some problem when running alasql in vm.

For example, in prepareFromData() function.

[agershun]

Thank you! What is the best replacement for x instanceof Array in this case?

[mathiasrw]

Array.isArray() is the best way to test if something is an array

So we can replcae with Array.isArray( ... ) in the 18 places we compare type with Array in the code

But I dont know if this solves the core of the problem: that we dont know if we can rely on any instanceOf - forexample when we if(tq instanceof yy.Table)

[xlvector]

Yes, this does not solve the core problem. The core problem is "require" method is passed from main context to vm context so objects in alasql is not created by vm context.

following is implement of instance of in js:

function instance_of(L, R) {
var O = R.prototype;
L = L.proto;
while (true) {
if (L === null)
return false;
if (O === L)// since L and R may be created by different vm, here use O == L may solve this problem.
return true;
L = L.proto;
}
}

However, core problem is L and R may be created by different vm, so we do not know if other function have same problem with instanceof .

I am still finding better ways.

[mathiasrw]

sounds good. I cant imagine we are the first library to front this when running in a js vm

[mathiasrw]

@xlvector Have you found out anything new?

[xlvector]

Sorry, I does not find better solution.

I can only restrict my users can only do read operation in vm because write operation will generate new objects.

[mathiasrw]

:-(

[nickdeis]

If we replace everything with named functions for the prototypes, we could just change everything from

if(x instanceof yy.Y){
  //do something 
}else if (x instanceof yy.Z){
  //do something else
}

to

switch(x.constructor.name){
  case "Y": 
    //do something;
    break;
  case "Z":
    //do something else;
    break;
  default:
   //default case
   break; 
}

This would probably be faster/result in less lines of code.
Unless this means that constructors can't transfer either, in which case we could just do something like

function Select(params){
  this.name = "Select";
  yy.extend(this,params);
}

NOTE: Function.prototype.name is immutable if the function is named

EDIT
Can confirm that

vm.runInNewContext("a.constructor.name === 'Array' ", {a:[]})

returns true.

[mathiasrw]

If we replace everything with named functions for the prototypes, we could just change everything

Sounds interesting - but I dont quite get it. what is it exactly that needs to be replaced with a named function?

[nickdeis]

instead of attaching prototypes directly to yy, you "name" them first.
A named function is when you construct a function as follows:

function Select(){

}

instead of

yy.Select = function(){}

You then attach Select to yy later, like so

yy.Select = Select;

The only reason I mention is that a direct consequence of using the syntax

export function Select(){}

For issue #445 is that all functions become named functions.

[mathiasrw]

@nickdeis I have paused changes in the ./src folder so we avoid a crazy merge with the ideas from #445

Woul it be easyer for you if I change all yy.Select = function(){} to function Select(){}+yy.Select = Select; in the ./src ?

[nickdeis]

I don't use alasql in nodejs (I have used it a lot in nashorn/rhino, the latter being the main reason I'm pursuing #445 ), I was just saw this ticket and found it interesting.

[mathiasrw]

Ahh - I get it - so with the ECMA6 way of using export function xxxx(){} they will be named and then we can change the checks accordingly - right?

[nickdeis]

Yep. You can even keep the if/else if/else structure, I just find switch more expressive.