Skip to content
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

Create empty array using destructuring #3294

Open
gengns opened this issue Mar 25, 2019 · 4 comments
Open

Create empty array using destructuring #3294

gengns opened this issue Mar 25, 2019 · 4 comments
Labels
internal-issue-created An internal Google issue has been created to track this GitHub issue triage-done Has been reviewed by someone on triage rotation.

Comments

@gengns
Copy link

gengns commented Mar 25, 2019

Closure Compiler does not create empty iterable arrays using destructuring.
[...Array(3)]

Given the input:

const html =
`<select>${[...Array(3)].reduce((acc, _, i) => acc += 
  `<option>${i}</option>`, '')}
</select>`

console.log(html)

A) We would expect the output to be:

'<select><option>0</option><option>1</option><option>2</option></select>'

https://jsfiddle.net/v9m7fzrd/

B) Instead, what we're getting is:

'<select></select>'


You can solve this issue in Closure Compiler using Array(3).fill() instead of [...Array(3)]

Closure Compiler does not warn you or gives you an error. We noticed this while checking the user interface.

@brad4d
Copy link
Contributor

brad4d commented Mar 25, 2019

Internal issue http://b/127803863 already exists

@brad4d brad4d added the internal-issue-created An internal Google issue has been created to track this GitHub issue label Mar 25, 2019
@brad4d
Copy link
Contributor

brad4d commented Mar 25, 2019

FYI, this isn't likely to change really soon.
Babel and TS do effectively the same thing.

The trouble is that AFAIK all methods for determining that an array is sparse are O(n).
So, we'd just have to always iterate over the array, which is an unreasonable time and space penalty for the much more common case where the array is not sparse.

@EatingW EatingW added the triage-done Has been reviewed by someone on triage rotation. label Mar 25, 2019
@gengns
Copy link
Author

gengns commented Mar 26, 2019

In the common case of "Array()" the array is not sparse, and Closure Compiler could transform that code into "Array().fill()".... as a human would do "by hand".

@brad4d
Copy link
Contributor

brad4d commented Mar 27, 2019

Array(10) does create a sparse array. It has a length of 10, but contains no actual properties.
This can create surprises even if no transpilation is involved at all.
e.g.

const allOnes = Array(10).map(() => 1);
console.log(allOnes[0]); // logs "undefined", not "1"

If the compiler always replaced Array(something) with Array(something).fill(undefined), we would break people who were depending on the array being sparse for behavior or to save memory.

If we tried to do the replacement only in the case where the value is immediately iterated over it would fix some cases, but not ones where the array was created farther away from the iteration. Also it would be even more confusing for users, since a valid refactoring of working code (where Array() is called close enough to recognize) could create broken code (where Array() is too far away for the compiler to recognize, and the problem happens again).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal-issue-created An internal Google issue has been created to track this GitHub issue triage-done Has been reviewed by someone on triage rotation.
Projects
None yet
Development

No branches or pull requests

3 participants