-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow ArrayLike as a spread source #7596
Comments
The arguments object is not an array type (though it resembles one), so the compiler error seems correct. Accept the arguments with a rest operator and they'll get converted to an array. function foo(...args) {
bar(...args);
} |
Now when you say it, I remember looking it up a dozen times now… Still, that functionality would be natural, especially given that |
@mhegazy I wasn't asking a question. This works in javascript, typescript fails to compile. |
it is not an array. quoting MDN:
it goes on to say:
so, function f() {
arguments.push(0); // Runtime error: Uncaught TypeError: arguments.push is not a function(…)
arguments.indexOf(0); // Runtime error: Uncaught TypeError: arguments.indexOf is not a
Array.isArray(arguments); // false
}
f(1,2); |
We're having a miscommunication. I am not saying it's an array, I'm stating the opposite, that it is not array, but it should be compatible with the spread operator. function foo(){
console.log(...arguments);
}
foo('bar', 'baz'); This throws a compile error, but produces a working code. Naturally |
i see. well. some of the transformation on a spread uses |
updating the title. |
'use strict';
function bar() {
console.log(arguments);
}
function foo() {
bar.call(this, ...arguments);
}
foo('bar', 'qat'); It fails in Firefox (but not in Chrome or Edge) and so would have to be transformed even when targeting ES6 with little or no value. It was the "magic" behaviours of |
as per the ES6 spec: http://www.ecma-international.org/ecma-262/6.0/#sec-createmappedargumentsobject, arguments is actually iterable. and spread allows iterables: http://www.ecma-international.org/ecma-262/6.0/#sec-runtime-semantics-arrayaccumulation. This means that we need to emit |
Cool... Firefox has a bug then (as it can't find the iterable interface on One challenge though is that doing that will leak |
An ArrayLike is not necessary an Iterable, nor vice versa. Since the native ES6 spread operator only allows Iterables, extending it to allow ArrayLikes would require type-dependent emit for --target ES6. |
Accepting PRs; this should be pretty straightforward. One big warning is that this is going to end up allowing |
Two more issues:
|
Not sure if there is a noticeable difference here for a normal program.
Do not think we can do type-directed emit.
Currently the TS compiler does not guard against these, and that has not been an issue in the past. e.g. |
I've extensively used the spread operator on declare var a: Derived1[], b: Derived2[], c: Base[];
c = a.concat(b); // Error
c = (<Base[]>a).concat(b); // Works but annoying
c = [...a, ...b]; // Works, and currently just transpiles down to .concat The proposed change would turn this into an inefficient double-copy of both arrays. So for performance sensitive applications this would make the common case of I do think it might be an improvement to allow ArrayLike in spread for non-concatenation uses only where it wouldn't force extra copying:
|
not sure i see the issue here. today this is emitted as: c = a.concat(b); under this proposal it would be emitted as: c = Array.prototype.concat.call(a, b); which should not be much different perf wise, and no extra copies needed. |
@mhegazy Everything prior to your comment implies that you intended to emit it as |
oh, no. that was not what i had in mind.. sorry if this was not clear. today we make an assumption that the value is an so these: c = [...a, ...b];
foo(...c);
[...x]
function f([a, ...x]) {} today emit as: c = a.concat(b);
foo.apply(void 0, c);
x.slice();
function f(_a) {
var a = _a[0], x = _a.slice(1);
} under this proposal would emit as: c = Array.prototype.concat.call(a, b);
foo.apply(void 0, c);
Array.prototyp.slice.call(x);
function f(_a) {
var a = _a[0], x = Array.prototype.slice.call(_a, 1);
} |
@mhegazy That doesn't work - .concat flattens out only actual
So allowing ArrayLike in spread, other than the trivial cases [...x] and f(...x), requires a double copy ( |
oh. did not think of that. duh.. back to the drawing board then :) |
|
This should be addressed by #12346 |
I believe #12346 did not address this, as I'm still seeing the same error in TS 2.6.2 |
@aaronresnick as mentioned in the PR text, when targeting ES3/ES5 you have to use |
So, to clarify, as of this day it still doesn't work, and the reason being - it doesn't work in FireFox. In the meantime, all our Node.js code in JavaScript uses Shouldn't support for spread of array-like objects become the default at this point, or at least default when building for the server-side? |
The spread operator only works on // from lib.es5.d.ts
interface ArrayLike<T> {
readonly length: number;
readonly [n: number]: T;
} Try this anywhere: [...{length: 0}];
// TypeError: ({length:0}) is not iterable
[...arguments] This issue should be closed as resolved or invalid. |
Valid -I have now realized that Invalid -One problem I had that led me to this issue was not being able to type spread arguments as function getProps<K extends ArrayLike<keyof User>>(...props: K) {
// above line gives this error at `...props: K` - A rest parameter must be of an array type.(2370)
} However there is a tiny workaround for this - function getProps2<K extends ArrayLike<keyof User>>(...props: K[number][]) {
// No error above!
} I hope this helps some. |
This works now |
When trying to pass caller arguments to another function with
...arguments
the compiler complains withType 'IArguments' is not an array type
. Version 1.8.9.The text was updated successfully, but these errors were encountered: