-
-
Notifications
You must be signed in to change notification settings - Fork 664
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(no-shadow-native-events): initial implementation #2558
base: master
Are you sure you want to change the base?
Conversation
Could you please fix the lint and test failures? |
@FloEdelmann Of course! Done! |
…lint-plugin-vue" until its released (#1877) Duplicate "no-shadow-native-events" from "eslint-plugin-vue" until it is released: - PR with `eslint-plugin-vue`: vuejs/eslint-plugin-vue#2558 - Tests can also be found in that PR - Official Rule proposal: vuejs/eslint-plugin-vue#2557 Relates to #1603
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't looked at the code yet, only at docs and tests. Apart from a few minor suggestions, those look good already!
{ | ||
filename: 'test.vue', | ||
code: ` | ||
<template> | ||
<div @click="$emit('click')"/> | ||
</template> | ||
<script setup> | ||
defineEmits(['click']) | ||
</script> | ||
`, | ||
errors: [ | ||
{ | ||
messageId: 'violation', | ||
line: 6, | ||
column: 20, | ||
endLine: 6, | ||
endColumn: 27 | ||
} | ||
] | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this report both the $emit()
call and the defineEmits
definition?
export type Emits2 = { | ||
(e: 'click' | 'bar', payload: string): void | ||
(e: 'keydown', payload: number): void | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this used for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for the test on line 1108 which ensures that imported types are also considered:
<script setup lang="ts">
import {Emits2 as Emits} from './test01'
const emit = defineEmits<Emits>()
</script>
…a96/eslint-plugin-vue into joca96/feat-no-shadow-native
Co-authored-by: Flo Edelmann <[email protected]>
Co-authored-by: Flo Edelmann <[email protected]>
{ | ||
messageId: 'violation', | ||
line: 4, | ||
column: 32, | ||
endLine: 4, | ||
endColumn: 37 | ||
}, | ||
{ | ||
messageId: 'violation', | ||
line: 4, | ||
column: 32, | ||
endLine: 4, | ||
endColumn: 37 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should only report once
Implementation of my proposed rule #2557 Rule Proposal: Disallow shadowing of native HTML event names.
The implementation is heavily based on require-explicit-emits.
The
dom-events.json
file is based on theEvents
interface of@vue/runtime-dom/dist/runtime-dom.d.ts
.