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

Support some TSTypes in the inferrer #14393

Merged
merged 1 commit into from Jun 29, 2022
Merged

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Mar 25, 2022

Q                       A
Fixed Issues? Fixes #14345
Patch: Bug Fix? Y
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

The inferrer is used by the spread transform and for-of transform to determine whether the argument / rhs is an array from its type annotations or static structure. This PR begins by adding TS tests to the inferrer and fixing bugs caught by the new test.

@JLHwung JLHwung added PR: Bug Fix 🐛 A type of pull request used for our changelog categories area: typescript labels Mar 25, 2022
@babel-bot
Copy link
Collaborator

babel-bot commented Mar 25, 2022

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

@@ -1,13 +0,0 @@
function foo() {
Copy link
Contributor Author

@JLHwung JLHwung Mar 25, 2022

Choose a reason for hiding this comment

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

This test has incorrect layout (should be one-folder deeper) so it is never run. I moved it to the spread plugin.

// Detect "var foo = Array()" calls so we can optimize for arrays vs iterables.
if (
init.isCallExpression() &&
init.get("callee").isIdentifier({ name: "Array" }) &&
Copy link
Contributor Author

@JLHwung JLHwung Mar 25, 2022

Choose a reason for hiding this comment

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

This is moved to the CallExpression handler.

@@ -47,7 +55,27 @@ export default function removeTypeDuplicates(
continue;
}

// TODO: add generic types
// todo: support merging tuples: number[]
Copy link
Contributor Author

@JLHwung JLHwung Mar 25, 2022

Choose a reason for hiding this comment

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

This is a follow-up to #11378.

for (const y of (b: Array<any>)) {}
function a(b: Array<any>) {
for (const y of b) {}
}
Copy link
Contributor Author

@JLHwung JLHwung Mar 25, 2022

Choose a reason for hiding this comment

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

This test is split to two tests because the former will be optimized but not the latter, which is very confusing.

packages/babel-plugin-transform-for-of/src/index.ts Outdated Show resolved Hide resolved
if (isTSTypeReference(node) && node.typeParameters) {
const name = getQualifiedName(node.typeName);

if (generics[name]) {
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo Apr 4, 2022

Choose a reason for hiding this comment

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

We should make sure that generics has a null prototype, otherwise this risks reading things from Object.prototype.

@JLHwung JLHwung force-pushed the fix-14345 branch 2 times, most recently from 9f4a46c to 25fa840 Compare Jun 10, 2022
* add ts inferer tests
* fix: support some TSTypes in inferrer
* Update packages/babel-plugin-transform-for-of/src/index.ts
* fix typing errors
* fix(inferer): allow {} prototype name in generics

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>
@nicolo-ribaudo nicolo-ribaudo merged commit 2f459ff into babel:main Jun 29, 2022
35 checks passed
@nicolo-ribaudo nicolo-ribaudo deleted the fix-14345 branch Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: typescript PR: Bug Fix 🐛 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants