Spread operator not applying to arguments object #111
Comments
|
I'm not sure how deep this bug goes so I've refactored my code using the following utilities in order to avoid the spread operator. //spread into a method call
const apply = ($this, fn, args) => $this[fn].apply($this, args);
// apply(String,'fromCharCode',args) ≈ String.fromCharCode(...args)
//spread into a function call
const enact = (fn, args) => fn.apply(undefined, args);
// enact(fetch,args) ≈ fetch(...args)
//spread into an array
const arr = (x) => Array.from(x);
// arr(x) ≈ [...x]
//spread into a constructor
const anew = (fn,args) => Reflect.construct(fn,args);
// anew(Request,args) ≈ new Request(...args) |
|
yeh, there was a bug of spread element. just fixed, and could you try if 0.5.9 works for you? |
|
@Siubaak |
|
I don't think I'll have time today but I'll try and set up a PR tomorrow. I'm thinking that using |
|
well. i think literally only iterable objects can be spread in an array according to the spec:
and HTMLCollection does have a
i can't reproduce your error of spreading edit: oh, i saw your code above and reproduced it. let me see what happened later on. |
|
Sorry I'm not trying to be annoying. Just following where my code goes. |
|
that's alright. no problem. sorry if i came across as too direct. all thoughts are welcome. :D i digged deep into it and found that's the problem of babel, instead of sval. when you use the code below: interpreter.run(`(${()=>{
function log(){ console.log(...Array.from(arguments)) }
log(1,2,3);
}})()`);it will be compiled to be the following result in the playground (by babel provided by jsbin): interpreter.run(`(${()=>{
function log(){ console.log(_toConsumableArray(Array.from(arguments))) }
log(1,2,3);
}})()`);then sval eventually runs the code like: interpreter.run(`
function log(){ console.log(_toConsumableArray(Array.from(arguments))) }
log(1,2,3);
`);and interpreter.run(`(()=>{
function log(){ console.log(...Array.from(arguments)) }
log(1,2,3);
})()`);edit: to be more specific, you could try the code below, and it will print the exact code in the jsbin console: const code = `(${()=>{
function log(){
console.log(...arguments);
/* [object Arguments] {
0: 1,
1: 2,
2: 3
} */
console.log(...Array.from(arguments));
/* 1
2
3 */
}
log(1,2,3);
}})()`
console.log(code);then you will see the code we run ends up being like: "(function () {
function log() {
window.runnerWindow.proxyConsole.log.apply(console, arguments);
/* [object Arguments] {
0: 1,
1: 2,
2: 3
} */
window.runnerWindow.proxyConsole.log.apply(console, _toConsumableArray(Array.from(arguments)));
/* 1
2
3 */
}
log(1, 2, 3);
})()" |
|
Ah I see. Things like this is why I append global scope manually to sval global scope like this. import Sval from './sval.js'
const interpreters = {
module : new Sval({
ecmaVer: 'latest',
sourceType: 'module',
sandBox: false,
}),
script : new Sval({
ecmaVer: 'latest',
sourceType: 'script',
sandBox: false,
}),
};
const svalGlobal = interpreters['module'].scope.context.globalThis.value;
const keys = [];
for(const key in globalThis){keys.push(key);}
const props = new Set(Object.getOwnPropertyNames(globalThis).concat(keys).concat(Object.getOwnPropertySymbols(globalThis)));
for(const prop of props){
svalGlobal[prop] = globalThis[prop];
}This could be a feature flag like how jsdom has Not that it would have helped here. |
|
if i'm not wrong, you don't have to do that, since sval has already import all variables from global context. see here. right now only the issue right here is that, for example, the code below interpreter.run(`${()=>console.log(...Array.from(arguments))}`);will be compiled to be (if using babel for compiling) function _toConsumableArray() { /* babel polyfill stuffs */ }
interpreter.run(`${()=>console.log(_toConsumableArray(Array.from(arguments)))}`);then during runtime, function _toConsumableArray() { /* babel polyfill stuffs */ }
interpreter.run(`()=>console.log(_toConsumableArray(Array.from(arguments)))`);this is the reason why interpreter.run(`()=>console.log(...Array.from(arguments))`); |
|
The reason I wrote the code using I've seen such outputs of babel that generate those utility functions. I know appending global scope objects wouldn't have helped here but I'm glad I mentioned it. My main use case is in cloudflare workers. Using sval is so far the only way to dynamically import remote scripts into cloudflare workers as all methods like |
|
Ah, i got your points. my bad for just mistaking ”not that” for “note that”. i will try to add |
|
I've just added support for
|
|
Yay it works. I initially got an error for(const prop of props){
svalGlobal[prop] = globalThis[prop];
}I removed that and it worked perfectly. I have one tiny suggestion that you might consider. In utils.ts starting at line 89 you have this. try { globalObj.Object = Object } catch (err) { /* empty */ }Rather than assign with const setProp = (obj,prop,value) => Object.defineProperty(obj,prop,{
value:value,
enumerable:true,
configurable:true,
writeable:true
});
try { setProp(globalObj,'Object',Object); } catch (err) { /* empty */ }It's essentially the same but bypasses any strange custom getter/setter logic. |
|
That's true. You're right. Some other logic should be handled like what you said as well. For example, using I think I'll review the whole codebase to fix all the similar situations in the future. |
The spread operator is not being properly applied to the arguments object. I use this often to chain function arguments.
Expected
console.log(...Array.from(arguments));andconsole.log(...arguments);to have the same output.Edit: for better highlighting
The text was updated successfully, but these errors were encountered: