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

Conversation

Amorites
Copy link
Contributor

Implement #1960 #1959

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.

lib/rules/prefer-type-emits-decl.js Outdated Show resolved Hide resolved
lib/rules/prefer-type-props-decl.js Outdated Show resolved Hide resolved
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
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.

docs/rules/README.md Outdated Show resolved Hide resolved
docs/rules/prefer-type-emits-decl.md Outdated Show resolved Hide resolved
lib/rules/prefer-type-emits-decl.js Outdated Show resolved Hide resolved
tests/lib/rules/prefer-type-emits-decl.js Outdated Show resolved Hide resolved
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
FloEdelmann
FloEdelmann reviewed Sep 15, 2022
View reviewed changes
docs/rules/define-emits-declaration.md Outdated Show resolved Hide resolved
docs/rules/define-props-declaration.md Outdated Show resolved Hide resolved
docs/rules/define-emits-declaration.md Outdated Show resolved Hide resolved
docs/rules/define-props-declaration.md Outdated Show resolved Hide resolved
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
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 :)

docs/rules/define-emits-declaration.md Outdated Show resolved Hide resolved
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?

docs/rules/define-props-declaration.md Outdated Show resolved Hide resolved
docs/rules/define-emits-declaration.md Outdated Show resolved Hide resolved
@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 FloEdelmann approved these changes

@ota-meshi ota-meshi ota-meshi approved these changes

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