JS Guide: NodeList prefer destructuring does not work OOB

[Josh68]

https://github.com/cloudfour/guides/tree/master/javascript#34-arrays-from-iterables.

In our current project, using Babel with @preset/env and .browserlistrc of IE 11 > 1%, using querySelectorAll to create a NodeList and then using [...myNodeList] does not transform the list to an array. Instead, it transpiles to [].concat(myNodeList), which is just the NodeList as element 0 of a new Array.

For the browser support in question, Array.from(myNodeList) works as expected, including in IE 11.

Not sure whether there's a way to tweak this setup to make it work, but my recommendation is to make this rule flexible, if we keep it at all

/cc @calebeby @gerardo-rodriguez @spaceninja @Paul-Hebert

[calebeby]

This is probably because we are using babel loose mode. I think if we switch to normal mode, it will actually iterate through the nodelist to create the array. The output code will be much larger, but I think it will work.

https://babeljs.io/docs/en/babel-plugin-transform-spread#options

If we do enable non-loose mode, we should probably do it for just this one plugin so it doesn't affect all the other babel plugins

[Josh68]

Thanks, @calebeby, I will give that a try. I was noticing loose mode, but didn't investigate further. In any case, our documentation should either loosen the requirement or explain what's needed to make this work. I will report back, and think the issue should remain open until we prove it out and update docs.

[Josh68]

Result is _toConsumableArray(this.querySelectorAll('[name="updates[]"]')) with the following config added to our babel.config.js:

plugins: [
    ...,
    '@babel/plugin-transform-spread',
    ...
]

I originally added the option loose:false to make sure it overrode the default, but that doesn't seem necessary. Just the plugin itself.

https://babeljs.io/docs/en/babel-plugin-transform-spread#loose

[gerardo-rodriguez]

our documentation should either loosen the requirement or explain what's needed to make this work

Totally! Whatever makes sense here! I started creating the JS guide document with the intention of it to not be the final, written-in-stone, sorry-don't-care-what-you-think documentation. :)

I like the idea of adding to the doc a "what's needed to make this work" section, if that makes sense. Now that we have a team of developers, I want this document to be the starting point so we can discuss, refine and make it better than where it's at now. This includes finishing it as I did not have time. 😅

[Josh68]

@gerardo-rodriguez, I'm not sure anyone knew, in this case, how this recommendation played with Babel, and I'm also still not clear (no time ATM) on how this plays without Babel or with a different .browserlistrc. My assumption is that it should work with Babel out of the mix.

In any case, we might even want to make a high-priority note in this doc that some recommendations that make assumptions about non-transpiled, ES-2015+ feature support might need to be tested out with a particular transpiler configuration. Then if we can flag particular items with notes about how to implement with a transpiler, even better.

[Josh68]

https://github.com/cloudfour/guides/pull/46 modified to address. I will close this issue in favor of new review of that PR.