phpv8 / v8js

V8 Javascript Engine for PHP — This PHP extension embeds the Google V8 Javascript Engine
http://pecl.php.net/package/v8js
MIT License
1.84k stars 200 forks source link

Clear variables between executeString calls #372

Closed redbullmarky closed 6 years ago

redbullmarky commented 6 years ago

If I create one instance of V8Js and run two executeString's on it, the second call has access to variables set in the first.

Without creating an entirely new instance of V8Js, and not knowing specifically WHICH variables may have been set in the first calls, is there a way to reset this data?

stesie commented 6 years ago

Sorry for not answering earlier ... and no, there is no such option. Especially since resetting variables is not enough, for example the executed code could have modified the prototypes of built-in objects.

So if you need separated execution environments, you need to go with multiple instances of V8Js class. Yet the second, third, etc. ones should come pretty cheap, both performance and memory wise.

redbullmarky commented 6 years ago

Hey no problem. We did manage to do some manual cleanup by assigning empty vars, etc, using just a single instance. i'll just feed back quickly the issues/solutions:

and other unrelated things, both related to compileString:

otherwise, i love V8 :-)

stesie commented 6 years ago

using multiple instances of V8 is INCREDIBLY slow

if you do that in a loop, yes agreed. I was refering to startup time vs. re-instantiation of a V8Js object, which still takes some time, but not as long as starting V8 initially. Anyways ...

I still haven't understood your use-case, maybe you'd like to elaborate a little more. Why do you keep re-executing the same code and are bothering with variables still in scope? I suppose you've got the javascript source code under control (or at least do not bother if the party "maliciously" modifies whatever prototypes), and you don't (want to) use global variables ... then why is variable scope a problem? If you want your local variables to "disappear", simply wrap within an IIFE like

$v8->executeString("(() => { let foo = 42; print(foo); })()");

... effects will be the same, and foo is out of scope again.

By the way, if you're using it to convert import/export data on a row-by-row base, and keep calling the same function continuously, you'll likely want to "compile" the function once and keep a reference to it on PHP side + directly invoke it from there like that:

php > $exporter = $v8->executeString("(row) => ({ data: row, blarg: 42 })");
php > var_dump($exporter("test"));
object(V8Object)#3 (2) {
  ["data"]=>
  string(4) "test"
  ["blarg"]=>
  int(42)
}
php > var_dump($exporter("another test"));
object(V8Object)#3 (2) {
  ["data"]=>
  string(12) "another test"
  ["blarg"]=>
  int(42)
}

... the function bound to $exporter obviously should be a little more involved, but I hope you get the idea. This way the same function is invoked multiple times, hence V8's JIT compiler should be able to kick in.

you cannot (or couldn't find a way) of overriding the main object_name.

I actually don't get that. What would you like to rename? The object named "PHP", which is provided by V8js?

it'd be ace if compileString could fully lint the code. that way, you could do pre-checks in systems that use V8 to provide code API's to users that would validate the code properly.

what do you mean with "fully lint"? After all V8 compiles the source, so it should find all syntax errors, etc. Yet it of course doesn't bind the variables, so if variables are missing at runtime, then that's runtime errors

it would be great if the return value of compileString wasn't a resource, but something that could be saved

V8 itself doesn't allow for scripts to be reused, hence if it would return a object, freeze/wakeup still wouldn't work...

Great that you like V8js BTW, always glad to hear :)

redbullmarky commented 6 years ago

so the system is effectively a Salesforce-like build-your-own CRM. And writing Javascript code offers extensibility for more advanced users. So using one example from this system, there are imports and exports of record data. And you can apply processors to either entire lines or individual fields - those are written in JS and executed per line.

So what I've done so far is created something like:

<?php class customjs extends V8Js { public static function instance() { static $instance; if (!$instance) { $instance = new self(); } return $instance; } }

but each call to executeString maintains the variables from the previous run. Not always bad, but considering you can add this to your JS code:

PHP = null;

which trashes the main object altogether, it's not so good. as for

you cannot (or couldn't find a way) of overriding the main object_name. so the problem we faced would have been ok if we could do (for example): var saved = myobjectname; // now run custom code; myobectname = saved;

what i meant was you cannot save a copy of the main object prior to running code, so that you can restore it afterwards (hence clearing the variables that may have been set, and also dealing with the PHP = null thing i mentioned above.

Otherwise, some little tips in your comment should really help with what we're trying to do, so thanks for taking the time to respond :-)

codejet commented 6 years ago

We also just ran into this issue with a file that contains the following JS code:

const { h } = require('preact');
const { render } = require('preact-render-to-string');
const Search = require('./web/ssr/js/search').default;

print(render(h(Search, { ...context.props })));

We need this file to be executed twice on a page, so that caused compileString to complain about h already being defined.

I thought that using const for the imports might be the sole issue here, but switching to let didn't help. But, to my surprise, switching to var did... I still don't get why let won't work.

Of course it would be nice to have an easy way to tell v8js to "forget" about prior executions instead of having to use multiple instances, but it sounds like this is nothing to be expected.

stesie commented 6 years ago

@codejet why not wrap the whole thing in a immediately invoked function expression? Then the const should be fine (as the variables fall out of scope after invocation)

codejet commented 6 years ago

Good point, @stesie. Somehow I didn't think further than var/let so far... 😅 Thanks!

Any explanation for why let was not working though?

stesie commented 6 years ago

@codejet well both executions are evaluated within the same context, hence the second let is a duplicate declaration within that scope, and according to section 13.3.1.1 of the spec (https://www.ecma-international.org/ecma-262/6.0/#sec-let-and-const-declarations) this is a syntax error (opposed to var declarations)

codejet commented 6 years ago

ahh, now i get it. my thinking was somehow completely wrong. of course it's not let h = ... and then h = ... subsequently, but both times let h = .... the interesting bit is that i wasn't even aware that you can call var h = ... twice within the same context. thanks!