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

Feat: update to Typescript 3.9 #1106

Merged
merged 14 commits into from Jun 9, 2020
Merged

Conversation

pikax
Copy link
Member

@pikax pikax commented May 3, 2020

Typescript 3.9.2 is currently breaking some types on defineComponent

This PR aims to fix those errors and also remove the need of TSD.

Closes #1106

@ktsn
Copy link
Member

ktsn commented May 4, 2020

We may not need to create DefineComponent type. See my comment at #626 (comment)

@pikax
Copy link
Member Author

pikax commented May 4, 2020

That's great! will try that out and update this :) Thanks!

@pikax pikax marked this pull request as ready for review May 8, 2020 09:12
@yyx990803
Copy link
Member

@pikax can you resolve the conflict in the lockfile?

package.json Outdated Show resolved Hide resolved
@@ -129,7 +130,7 @@ describe('with object props', () => {
expectType<ExpectedProps['eee']>(this.eee)
expectType<ExpectedProps['fff']>(this.fff)

// props on `this` should be readonly
// @ts-expect-error props on `this` should be readonly
expectError((this.a = 1))
Copy link
Contributor

Choose a reason for hiding this comment

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

is expectError necessery here? maybe it's enough to do

// @ts-expect-error props on `this` should be readonly
this.a = 1

?

Copy link
Member Author

Choose a reason for hiding this comment

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

I kept the expectError to keep the same interface as before, felt a bit better to me, is not necessary to have it

@@ -0,0 +1,10 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this build config? (What are we testing with this?)

Copy link
Member Author

Choose a reason for hiding this comment

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

This tests the output files, to make sure our output typings are correct. Because of the usage of @internal we might hide some apis, using this config will make the dependencies to resolve to the publish dirs instead of the src code

Copy link
Member

@ktsn ktsn left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@yyx990803
Copy link
Member

Will merge first thing next week!

@yyx990803 yyx990803 merged commit 97dedeb into vuejs:master Jun 9, 2020
@pikax pikax deleted the feat/ts3.9-rc_2 branch October 5, 2020 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants