Skip to content

Implement iterableIsArray and arrayLikeIsIterable assumptions#12489

Merged
nicolo-ribaudo merged 3 commits into
babel:feat-7.13.0/babel-core-featuresfrom
nicolo-ribaudo:assumption/array
Dec 14, 2020
Merged

Implement iterableIsArray and arrayLikeIsIterable assumptions#12489
nicolo-ribaudo merged 3 commits into
babel:feat-7.13.0/babel-core-featuresfrom
nicolo-ribaudo:assumption/array

Conversation

@nicolo-ribaudo

@nicolo-ribaudo nicolo-ribaudo commented Dec 11, 2020

Copy link
Copy Markdown
Member
Q                       A
Major: Breaking Change? Yes
Tests Added + Pass? Yes
License MIT

Main PR: #12219
RFC: babel/rfcs#5

Picking the correct defaults was hard in this PR, so please check if what I did makes sense.

@nicolo-ribaudo nicolo-ribaudo added PR: New Feature 🚀 A type of pull request used for our changelog categories area: assumptions labels Dec 11, 2020
@babel-bot

babel-bot commented Dec 11, 2020

Copy link
Copy Markdown
Collaborator

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/34972/

@codesandbox-ci

Copy link
Copy Markdown

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 1e64d8f:

Sandbox Source
babel-repl-custom-plugin Configuration
babel-plugin-multi-config Configuration

- transform-destructuring
- transform-for-of
- transform-spread
- transform-destructuring
- transform-for-of
- transform-spread

@JLHwung JLHwung left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we going to rename allowArrayLike to arrayLikeIsIterable in Babel 8?

@nicolo-ribaudo

Copy link
Copy Markdown
Member Author

If we keep it yes; I'm not sure if it's it worth to keep it around given the new assumptions.arrayLikeIsIterable.

@nicolo-ribaudo nicolo-ribaudo merged commit a275e00 into babel:feat-7.13.0/babel-core-features Dec 14, 2020
@nicolo-ribaudo nicolo-ribaudo deleted the assumption/array branch December 14, 2020 22:38
const arrayOnlySpread = loose;
const iterableIsArray = api.assumption("iterableIsArray") ?? options.loose;
const arrayLikeIsIterable =
options.allowArrayLike ?? api.assumption("arrayLikeIsIterable");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this backwards?

const { loose, allowArrayLike } = options;
const iterableIsArray = api.assumption("iterableIsArray") ?? options.loose;
const arrayLikeIsIterable =
options.allowArrayLike ?? api.assumption("arrayLikeIsIterable");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto backwards.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for swapping the order here is that, while loose is generic (and thus it's good to keep it as a fallback value), this option was explicitly designed for this assumption and it has a quite specific name. Thus I followed the "locality principle": since they have the same specificity, the option closer to the plugin has the precedence.

I wasn't 100% sure of doing this, so I'm open to change it to match loose's behavior if no one else thinks that options.allowArrayLike should take precedence.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense to me that when options.allowArrayLike is specified, that it should take precedence over api.assumption("arrayLikeIsIterable").

nicolo-ribaudo added a commit to nicolo-ribaudo/babel that referenced this pull request Jan 12, 2021
nicolo-ribaudo added a commit to nicolo-ribaudo/babel that referenced this pull request Feb 4, 2021
nicolo-ribaudo added a commit to nicolo-ribaudo/babel that referenced this pull request Feb 10, 2021
nicolo-ribaudo added a commit to nicolo-ribaudo/babel that referenced this pull request Feb 11, 2021
nicolo-ribaudo added a commit that referenced this pull request Feb 16, 2021
- `mutableTemplateObject` and `ignoreToPrimitiveHint` (#12408)
- `setClassMethods` (#12407)
- `setComputedProperties` (#12490)
- `ignoreFunctionLength` (#12491)
- `noDocumentAll` (#12481)
- `iterableIsArray` and `arrayLikeIsIterable` (#12489)
- `pureGetters` (#12504)
- `skipForOfIteratorClosing` (#12496)
- `objectRestNoSymbols`, `setSpreadProperties` and `pureGetters` (#12505)
- `noNewArrows` (#12613, #12793)
- `setPublicClassFields` and `privateFieldsAsProperties` (#12497)
- `constantReexports` and `enumerableModuleMeta` (#12618)
- `constantSuper`, `superIsCallableConstructor` and `noClassCalls` (#12726)

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
Co-authored-by: Huáng Jùnliàng <JLHwung@users.noreply.github.com>
nicolo-ribaudo added a commit that referenced this pull request Feb 16, 2021
- `mutableTemplateObject` and `ignoreToPrimitiveHint` (#12408)
- `setClassMethods` (#12407)
- `setComputedProperties` (#12490)
- `ignoreFunctionLength` (#12491)
- `noDocumentAll` (#12481)
- `iterableIsArray` and `arrayLikeIsIterable` (#12489)
- `pureGetters` (#12504)
- `skipForOfIteratorClosing` (#12496)
- `objectRestNoSymbols`, `setSpreadProperties` and `pureGetters` (#12505)
- `noNewArrows` (#12613, #12793)
- `setPublicClassFields` and `privateFieldsAsProperties` (#12497)
- `constantReexports` and `enumerableModuleMeta` (#12618)
- `constantSuper`, `superIsCallableConstructor` and `noClassCalls` (#12726)

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
Co-authored-by: Huáng Jùnliàng <JLHwung@users.noreply.github.com>
nicolo-ribaudo added a commit that referenced this pull request Feb 21, 2021
- `mutableTemplateObject` and `ignoreToPrimitiveHint` (#12408)
- `setClassMethods` (#12407)
- `setComputedProperties` (#12490)
- `ignoreFunctionLength` (#12491)
- `noDocumentAll` (#12481)
- `iterableIsArray` and `arrayLikeIsIterable` (#12489)
- `pureGetters` (#12504)
- `skipForOfIteratorClosing` (#12496)
- `objectRestNoSymbols`, `setSpreadProperties` and `pureGetters` (#12505)
- `noNewArrows` (#12613, #12793)
- `setPublicClassFields` and `privateFieldsAsProperties` (#12497)
- `constantReexports` and `enumerableModuleMeta` (#12618)
- `constantSuper`, `superIsCallableConstructor` and `noClassCalls` (#12726)

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
Co-authored-by: Huáng Jùnliàng <JLHwung@users.noreply.github.com>
@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 Mar 17, 2021
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Mar 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area: assumptions outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: New Feature 🚀 A type of pull request used for our changelog categories

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants