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

Remove backward compatibility for union types #11468

Merged
merged 2 commits into from Apr 26, 2020
Merged

Remove backward compatibility for union types #11468

merged 2 commits into from Apr 26, 2020

Conversation

Beraliv
Copy link
Contributor

@Beraliv Beraliv commented Apr 22, 2020

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

Several requests regards TypeScript union types saved backward compatibility
Here I remove them for major update (Babel 8)

Related issue:

Related PRs:

@@ -95,15 +95,11 @@ function getTypeAnnotationBindingConstantViolations(binding, path, name) {
return;
}

if (t.isTSTypeAnnotation(types[0]) && t.createTSUnionType) {
if (t.isTSTypeAnnotation(types[0])) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we always have createTSUnionType in Babel 8

}

return t.createUnionTypeAnnotation(types);
return t.createFlowUnionType(types);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We always have createFlowUnionType in Babel 8 as well 👌

@JLHwung JLHwung added the PR: Polish 💅 A type of pull request used for our changelog categories label Apr 22, 2020
@babel-bot
Copy link
Collaborator

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

@JLHwung JLHwung added PR: Breaking Change 💥 A type of pull request used for our changelog categories for next major release and removed PR: Polish 💅 A type of pull request used for our changelog categories labels Apr 22, 2020
@Beraliv
Copy link
Contributor Author

Beraliv commented Apr 22, 2020

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

Test case can be found here

@Beraliv
Copy link
Contributor Author

Beraliv commented Apr 22, 2020

🧪e2e-babel-old-version tests are failed expectedly as I break backward compatibility ✅

@Beraliv
Copy link
Contributor Author

Beraliv commented Apr 26, 2020

@nicolo-ribaudo do I need anything to help with before merging?

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.

No, this looks good to me!

If you want to make CI pass you can rebase the PR, but it's not really needed since the failing test has been disabled on next-8-dev.

@existentialism
Copy link
Member

@Beraliv thanks for the PRs!

@nicolo-ribaudo nicolo-ribaudo merged commit 453ec42 into babel:next-8-dev Apr 26, 2020
@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 Jul 27, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 27, 2020
@JLHwung JLHwung added PR: Internal (next major) 🏠 A type of pull request used for our changelog categories for next major release and removed PR: Breaking Change 💥 A type of pull request used for our changelog categories for next major release labels Aug 9, 2023
@JLHwung JLHwung modified the milestone: v8.0.0 Aug 9, 2023
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 PR: Internal (next major) 🏠 A type of pull request used for our changelog categories for next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants