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

Babel loose mode is unsafe with array spread operator #8298

Closed
TrySound opened this issue Jul 10, 2018 · 6 comments
Closed

Babel loose mode is unsafe with array spread operator #8298

TrySound opened this issue Jul 10, 2018 · 6 comments
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue

Comments

@TrySound
Copy link
Contributor

Bug Report

I just went though two bugs with array spread operator.

Spread of array constructor call

// input
[...Array(100)] // array of 100 undefined values
// output
Array(100).concat() // array of 100 empty values

Spread of string

// input
[...'abc'] // array of chars
// output
'abc'.concat() // "abc' string

Expected behavior/code
I think this loose mode is too loose and could be slightly improved with small helpers. This things are the only problems I had with loose mode in a year.

Babel Configuration (.babelrc, package.json, cli command)

{
  "presets": [["env", { "loose": true }]]
}

Environment

  • Babel version(s): v7.0.0-beta.34

Possible Solution
Make loose mode for spread operator slightly stricter.

@babel-bot
Copy link
Collaborator

Hey @TrySound! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

@Andarist
Copy link
Member

Well, this is a cost of using loose mode. It's not always spec-compliant and such cases might bite you. I don't think holey arrays are popular enough to warrant introducing toConsumableArray in the loose mode.

Maybe we could try to track if array might be holey and use this helper IF we know that it is - it would be limited only to the simplest situations though - analyzing runtime values with babel is not easy.

We could similarly special case string literals and convert them to 'abc'.split('') - but that would also only cover simplest situations.

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Jul 10, 2018

Loose mode assumes that your code behaves "in the most obvious way": Babel assumes that you are only using array spread with "normal" (e.g. not holey) arrays.

@kasperpeulen
Copy link

kasperpeulen commented Mar 8, 2019

@nicolo-ribaudo

Loose mode assumes thematic your code behaves "in the most obvious way": Babel assumes that you are only using array spread with "normal" (e.g. not holey) arrays.

Spread was meant for all Iterable's, not only Array's. It should work with String, Set, Map, Generator etc.

export const locationChange = ({ pathname, search }) => ({
  type: LOCATION_CHANGE,
  mutation: {
    pathname,
    search,
    params:  new URLSearchParams(search)
        |> map(([key, value]) => ({ [key]: value }))
        // Array.from is needed because we use babel in loose mode
        |> it => Object.assign({}, ...Array.from(it)),
  },
});

Here Array.from is needed in loose mode, because map returns a Generator:

export const map = transform =>
  function*(iterable) {
    for (const item of iterable) {
      yield transform(item);
    }
  };

I wouldn't call it obvious that spread doesn't work for all Iterable's.

@fdanielsen
Copy link

@kasperpeulen Agreed. Just hit this myself upgrading to Babel v7, where NodeList does not get converted to an array in loose mode.

@nicolo-ribaudo
Copy link
Member

I'm closing this for two reasons:

@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label May 4, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue
Projects
None yet
Development

No branches or pull requests

6 participants