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

output aliased types in typescript declarations #8629

Merged
merged 1 commit into from Nov 5, 2018

Conversation

43081j
Copy link
Contributor

@43081j 43081j commented Sep 4, 2018

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

We weren't outputting aliased types in our isXX assertion methods so we would be losing type information (returning boolean instead).

@babel-bot
Copy link
Collaborator

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

1 similar comment
@babel-bot
Copy link
Collaborator

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

Copy link
Member

@xtuc xtuc left a comment

Choose a reason for hiding this comment

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

Please add tests

@43081j
Copy link
Contributor Author

43081j commented Sep 10, 2018

Do we have any tests for the generators at all? I'm happy to write some but wasn't sure if we already had any I could extend.

@xtuc
Copy link
Member

xtuc commented Sep 10, 2018

Yes that's right, sorry.

@xtuc
Copy link
Member

xtuc commented Sep 10, 2018

I think it doesn't make sense to add tests on the generator, we could instead just commit the file, but the point is to avoid that.

@43081j
Copy link
Contributor Author

43081j commented Sep 10, 2018

No worries.

It's possible to write tests but they would be type tests rather than unit. Similar to how definitelytyped works. You have a usage file and assert that it compiles.

Maybe something for the future.

@43081j
Copy link
Contributor Author

43081j commented Oct 8, 2018

Could we get some eyes on this and possibly merge it? Its a very small change but we can't use the shipped TS definitions until then

@nicolo-ribaudo nicolo-ribaudo merged commit b95cbc4 into babel:master Nov 5, 2018
@43081j 43081j deleted the ts-aliases branch November 6, 2018 08:43
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 4, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: typescript outdated A closed issue/PR that is archived due to age. Recommended to make a new issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants