search

summer4096 / dockerscript

Write your Dockerfile in javascript!
MIT License
22 stars 3 forks source link

Use of sandboxed-module breaks asynchronous code. #2

Open tolmasky opened 7 years ago

tolmasky commented 7 years ago

If you have code like this:

var t = require("./x");

setTimeout(function()
{
    console.log(t === require("./x"));
}, 1000);

The second require appears to be not sandboxed while the first is. Not sure, but they are certainly different and this results false.

tolmasky commented 7 years ago

Changing singleOnly to false seems to resolve this issue.

summer4096 commented 7 years ago

I think that was a concession to make babel work. Babel probably isn't necessary for this anymore. PR?

tolmasky commented 7 years ago

Just to clarify, would you like me to just change singleOnly to false, or also remove babel?

summer4096 commented 7 years ago

Both. If singleOnly is false but babel is still in, it'll be transpiling every module you require, which might end up being very bad.

tolmasky commented 7 years ago

As a question, why is sandboxed-module used at all? Is there an actual thing being protected against? In my experiments it is very difficult to get it to behave correctly using it at all, but if I just require the file it's fine.

summer4096 commented 7 years ago

The whole idea was to allow template tag syntax before it was supported in node. Using sandboxed-module allowed that through babel, but now that node supports it, there isn't much reason to keep using either sandboxed-module or babel. I'm not exactly sure how to pull it out but, if you can, go for it.

tolmasky commented 7 years ago

OK, I've removed it. Again, I bumped up the version since it's a breaking change, but not sure how to handle that.