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

Add type-based declaration rule of defineProps and defineEmits #1968

Merged
merged 20 commits into from
Sep 16, 2022
Merged

Add type-based declaration rule of defineProps and defineEmits #1968

merged 20 commits into from
Sep 16, 2022
+663 −48

Conversation

Amorites
Copy link
Contributor

Implement #1960 #1959

@Amorites
Add vue/prefer-type-props-decl rule
2506c80
@Amorites
Add rule

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
5fa4d5c
@Amorites
chore: Related Rules

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
dd6f19d
ota-meshi
ota-meshi requested changes Sep 13, 2022
View reviewed changes
Copy link
Member

@ota-meshi ota-meshi left a comment

Choose a reason for hiding this comment

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

Thank you for this PR! Almost looks good!! I have a minor change request.

Amorites and others added 2 commits September 13, 2022 10:42
@Amorites @ota-meshi
Update lib/rules/prefer-type-emits-decl.js
a0614b6 
Co-authored-by: Yosuke Ota <otameshiyo23@gmail.com>
@Amorites @ota-meshi
Update lib/rules/prefer-type-props-decl.js
1aba686 
Co-authored-by: Yosuke Ota <otameshiyo23@gmail.com>
ota-meshi
ota-meshi approved these changes Sep 13, 2022
View reviewed changes
Copy link
Member

@ota-meshi ota-meshi left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@Amorites
fix: remove ts eslint-code-block & improve related rules link

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
d5ff0b4
@Amorites
Copy link
Contributor Author

Found Parsing error in docs editor and removed eslint-code-block. Maybe I could look into a solution and get it back later.

@ota-meshi
Copy link
Member

Good point. The current documentation site does not parse typescript.

This was linked to issues Sep 13, 2022
Force TypeScript only defineProps #1959
Closed
Force TypeScript only defineEmits #1960
Closed
FloEdelmann
FloEdelmann requested changes Sep 13, 2022
View reviewed changes
Copy link
Member

@FloEdelmann FloEdelmann left a comment

Choose a reason for hiding this comment

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

Thanks for the implementation! Everything looks fine, but I have a few suggestions.

Amorites and others added 4 commits September 14, 2022 20:04
@Amorites @FloEdelmann
Update lib/rules/prefer-type-emits-decl.js
6545e85 
Co-authored-by: Flo Edelmann <florian-edelmann@online.de>
@Amorites @FloEdelmann
Update tests/lib/rules/prefer-type-emits-decl.js
9dd43d8 
Co-authored-by: Flo Edelmann <florian-edelmann@online.de>
@Amorites
chore: rename rule name
624396e
@Amorites
feat: implement renamed rules
ae2398c
@ota-meshi ota-meshi mentioned this pull request Sep 15, 2022
Merged
@Amorites
update rule docs

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
4619113
@Amorites
Merge remote-tracking branch 'origin/master' into prefer-type-props-decl
8b16330
@Amorites
Add eslint-code-block

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
a4030a5
@Amorites
fix lint issues
7ff9ded
FloEdelmann
FloEdelmann reviewed Sep 15, 2022
View reviewed changes
Amorites and others added 4 commits September 15, 2022 19:15
@Amorites @FloEdelmann
Update docs/rules/define-emits-declaration.md
ee72388 
Co-authored-by: Flo Edelmann <florian-edelmann@online.de>
@Amorites @FloEdelmann
Update docs/rules/define-props-declaration.md
36f9996 
Co-authored-by: Flo Edelmann <florian-edelmann@online.de>
@Amorites @FloEdelmann
Update docs/rules/define-emits-declaration.md

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
763fb88 
Co-authored-by: Flo Edelmann <florian-edelmann@online.de>
@Amorites @FloEdelmann
Update docs/rules/define-props-declaration.md
aa16805 
Co-authored-by: Flo Edelmann <florian-edelmann@online.de>
FloEdelmann
FloEdelmann requested changes Sep 15, 2022
View reviewed changes
Copy link
Member

@FloEdelmann FloEdelmann left a comment

Choose a reason for hiding this comment

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

One more final thing :)

@Amorites
add runtime option example

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
0adae66
FloEdelmann
FloEdelmann approved these changes Sep 15, 2022
View reviewed changes
Copy link
Member

@FloEdelmann FloEdelmann left a comment

Choose a reason for hiding this comment

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

Looks good to me now. Thanks for the changes! (I'll apply the two remaining docs changes myself.)

@ota-meshi can you please clear the cache in Netlify for https://deploy-preview-1968--eslint-plugin-vue.netlify.app/rules/define-props-declaration.html, so we can have a look at the examples there?

@ota-meshi
Copy link
Member

I have cleared the cache and retried. You can see it now 👍
https://deploy-preview-1968--eslint-plugin-vue.netlify.app/rules/define-props-declaration.html

FloEdelmann
FloEdelmann approved these changes Sep 16, 2022
View reviewed changes
Copy link
Member

@FloEdelmann FloEdelmann left a comment

Choose a reason for hiding this comment

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

Looks like this is ready to merge then :)

@ota-meshi ota-meshi merged commit 4383a86 into vuejs:master Sep 16, 2022
@mesqueeb
Copy link
Contributor

@ota-meshi defineProps example has some defineEmits code

@Amorites
Copy link
Contributor Author

Sorry about that, I made a new PR to fix it. #1977

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FloEdelmann FloEdelmann

@ota-meshi ota-meshi

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Force TypeScript only defineEmits Force TypeScript only defineProps
4 participants
@Amorites @ota-meshi @mesqueeb @FloEdelmann