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.

Sign up for GitHub

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

Jump to bottom

fix: Don't throw in getTypeAnnotation when using TS+inference #15383

Merged
merged 2 commits into from Jan 15, 2024
Merged

fix: Don't throw in getTypeAnnotation when using TS+inference #15383

merged 2 commits into from Jan 15, 2024

Conversation

liuxingbaoyu
Copy link
Member

@liuxingbaoyu liuxingbaoyu commented Jan 30, 2023 •
edited

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

@liuxingbaoyu liuxingbaoyu added PR: Bug Fix 🐛 A type of pull request used for our changelog categories area: typescript i: regression pkg: traverse labels Jan 30, 2023
@babel-bot
Copy link
Collaborator

babel-bot commented Jan 30, 2023 •
edited

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

nicolo-ribaudo
nicolo-ribaudo reviewed Jan 30, 2023
View reviewed changes
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

createUnionType could not return undefined, because the if/else checks are not exhaustive anymore. Could we either:

  1. Update its type definition to function createUnionType(types: Array<t.FlowType | t.TSType>) : t.FlowType | t.TSType | undefined
  2. Update its type definition to funcion createUntionType<T extends Array<t.FlowType> | Array<t.TSType>>(types: T): T[number], and only call it if all the types are from the same type system

I slightly prefer (2), but (1) is also fine.

@liuxingbaoyu
Copy link
Member Author

Makes sense!
I also like 2, but I find it is called multiple places.
I thought of two ways

  1. Modify createUnionType to allow returning undefined
  2. Add maybeCreateUnionType and put the check inside
    These functions are not exported, so we can safely modify them.

@liuxingbaoyu liuxingbaoyu force-pushed the fix-ts-and-flow-getTypeAnnotation branch from 0d0b04f to c9277f4 Compare November 11, 2023 05:33
@nicolo-ribaudo
Copy link
Member

Thanks for rebasing, did you go with any of the suggestions in #15383 (review)?

@liuxingbaoyu
Copy link
Member Author

Yes, I chose the first one. :)

@nicolo-ribaudo nicolo-ribaudo changed the title fix: getTypeAnnotation unexpectedly throws an exception fix: Don't throw in getTypeAnnotation when using TS+inference Jan 15, 2024
@nicolo-ribaudo nicolo-ribaudo merged commit 1200542 into babel:main Jan 15, 2024
48 checks passed
@liuxingbaoyu liuxingbaoyu mentioned this pull request Jan 25, 2024
Closed
1 task
Vylpes pushed a commit to Vylpes/random-bunny that referenced this pull request Apr 10, 2024
@Vylpes
Update dependency @babel/traverse to v7.23.9 (#142)
bfb2e73 
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [@babel/traverse](https://babel.dev/docs/en/next/babel-traverse) ([source](https://github.com/babel/babel)) | resolutions | patch | [`7.23.7` -> `7.23.9`](https://renovatebot.com/diffs/npm/@babel%2ftraverse/7.23.7/7.23.9) |

---

### Release Notes

<details>
<summary>babel/babel (@&#8203;babel/traverse)</summary>

### [`v7.23.9`](https://github.com/babel/babel/blob/HEAD/CHANGELOG.md#v7239-2024-01-25)

[Compare Source](babel/babel@v7.23.7...v7.23.9)

##### 🐛 Bug Fix

-   `babel-helper-transform-fixture-test-runner`, `babel-plugin-transform-function-name`, `babel-plugin-transform-modules-systemjs`, `babel-preset-env`
    -   [#&#8203;16225](babel/babel#16225) fix: `systemjs` re-traverses helpers ([@&#8203;liuxingbaoyu](https://github.com/liuxingbaoyu))
-   `babel-helper-create-class-features-plugin`, `babel-plugin-proposal-decorators`
    -   [#&#8203;16226](babel/babel#16226) Improve decorated private method check ([@&#8203;JLHwung](https://github.com/JLHwung))
-   `babel-plugin-proposal-decorators`, `babel-plugin-transform-async-generator-functions`, `babel-plugin-transform-runtime`, `babel-preset-env`
    -   [#&#8203;16224](babel/babel#16224) Properly sort `core-js@3` imports ([@&#8203;nicolo-ribaudo](https://github.com/nicolo-ribaudo))
-   `babel-traverse`
    -   [#&#8203;15383](babel/babel#15383) fix: Don't throw in `getTypeAnnotation` when using TS+inference ([@&#8203;liuxingbaoyu](https://github.com/liuxingbaoyu))
-   Other
    -   [#&#8203;16210](babel/babel#16210) \[eslint] Fix `no-use-before-define` for class ref in fields ([@&#8203;nicolo-ribaudo](https://github.com/nicolo-ribaudo))

##### 🏠 Internal

-   `babel-core`, `babel-parser`, `babel-template`
    -   [#&#8203;16222](babel/babel#16222) Migrate `eslint-parser` to cts ([@&#8203;liuxingbaoyu](https://github.com/liuxingbaoyu))
-   `babel-types`
    -   [#&#8203;16213](babel/babel#16213) Remove `@babel/types` props that are not produced by the parser ([@&#8203;liuxingbaoyu](https://github.com/liuxingbaoyu))

##### 🏃‍♀️ Performance

-   `babel-parser`
    -   [#&#8203;16072](babel/babel#16072) perf: Improve parser performance for typescript ([@&#8203;liuxingbaoyu](https://github.com/liuxingbaoyu))

##### 🔬 Output optimization

-   `babel-helper-create-class-features-plugin`, `babel-plugin-proposal-decorators`, `babel-plugin-proposal-destructuring-private`, `babel-plugin-proposal-pipeline-operator`, `babel-plugin-transform-class-properties`, `babel-plugin-transform-class-static-block`, `babel-plugin-transform-new-target`, `babel-plugin-transform-parameters`, `babel-plugin-transform-private-methods`, `babel-preset-env`
    -   [#&#8203;16218](babel/babel#16218) Improve temporary variables for decorators ([@&#8203;liuxingbaoyu](https://github.com/liuxingbaoyu))
-   `babel-helpers`, `babel-plugin-proposal-explicit-resource-management`, `babel-runtime-corejs2`, `babel-runtime-corejs3`, `babel-runtime`
    -   [#&#8203;15959](babel/babel#15959) Improve output of `using` ([@&#8203;liuxingbaoyu](https://github.com/liuxingbaoyu))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4wLjAiLCJ1cGRhdGVkSW5WZXIiOiIzNy4wLjAiLCJ0YXJnZXRCcmFuY2giOiJkZXZlbG9wIn0=-->

Reviewed-on: https://gitea.vylpes.xyz/RabbitLabs/random-bunny/pulls/142
Reviewed-by: Vylpes <ethan@vylpes.com>
Co-authored-by: Renovate Bot <renovate@vylpes.com>
Co-committed-by: Renovate Bot <renovate@vylpes.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 Apr 16, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@nicolo-ribaudo nicolo-ribaudo nicolo-ribaudo approved these changes

Assignees
No one assigned
Labels
area: typescript i: regression outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: traverse PR: Bug Fix 🐛 A type of pull request used for our changelog categories
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: TypeScript compilation throws error on correct program (type inference on array spread syntax?)
3 participants
@liuxingbaoyu @babel-bot @nicolo-ribaudo