Skip to content

Commit

Permalink
feat: improve component event handler type (#314)
Browse files Browse the repository at this point in the history
* feat: improve component event handler type

* fix: test

* fix: test

* chore: refactor
  • Loading branch information
ota-meshi committed Apr 26, 2023
1 parent f13e307 commit 96a72a5
Show file tree
Hide file tree
Showing 33 changed files with 10,578 additions and 206 deletions.
5 changes: 5 additions & 0 deletions .changeset/clean-taxis-rule.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"svelte-eslint-parser": minor
---

feat: improve component event handler type
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,9 @@
"semver": "^7.3.5",
"string-replace-loader": "^3.0.3",
"svelte": "^3.57.0",
"svelte2tsx": "^0.6.11",
"typescript": "~5.0.0",
"typescript-eslint-parser-for-extra-files": "^0.3.0",
"vue-eslint-parser": "^9.0.0"
},
"publishConfig": {
Expand Down
101 changes: 65 additions & 36 deletions src/parser/converts/attr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -346,52 +346,81 @@ function buildEventHandlerType(
elementName: string,
eventName: string
) {
const nativeEventHandlerType = [
`(e:`,
/**/ `'${eventName}' extends infer EVT`,
/**/ /**/ `?EVT extends keyof HTMLElementEventMap`,
/**/ /**/ /**/ `?HTMLElementEventMap[EVT]`,
/**/ /**/ /**/ `:CustomEvent<any>`,
/**/ /**/ `:never`,
`)=>void`,
].join("");
const nativeEventHandlerType = `(e:${conditional({
check: `'${eventName}'`,
extends: `infer EVT`,
true: conditional({
check: `EVT`,
extends: `keyof HTMLElementEventMap`,
true: `HTMLElementEventMap[EVT]`,
false: `CustomEvent<any>`,
}),
false: `never`,
})})=>void`;
if (element.type !== "SvelteElement") {
return nativeEventHandlerType;
}
if (element.kind === "component") {
// `@typescript-eslint/parser` currently cannot parse `*.svelte` import types correctly.
// So if we try to do a correct type parsing, it's argument type will be `any`.
// A workaround is to inject the type directly, as `CustomEvent<any>` is better than `any`.

// const componentEvents = `import('svelte').ComponentEvents<${elementName}>`;
// return `(e:'${eventName}' extends keyof ${componentEvents}?${componentEvents}['${eventName}']:CustomEvent<any>)=>void`;

return `(e:CustomEvent<any>)=>void`;
const componentEventsType = `import('svelte').ComponentEvents<${elementName}>`;
return `(e:${conditional({
check: `0`,
extends: `(1 & ${componentEventsType})`,
// `componentEventsType` is `any`
// `@typescript-eslint/parser` currently cannot parse `*.svelte` import types correctly.
// So if we try to do a correct type parsing, it's argument type will be `any`.
// A workaround is to inject the type directly, as `CustomEvent<any>` is better than `any`.
true: `CustomEvent<any>`,
// `componentEventsType` has an exact type.
false: conditional({
check: `'${eventName}'`,
extends: `infer EVT`,
true: conditional({
check: `EVT`,
extends: `keyof ${componentEventsType}`,
true: `${componentEventsType}[EVT]`,
false: `CustomEvent<any>`,
}),
false: `never`,
}),
})})=>void`;
}
if (element.kind === "special") {
if (elementName === "svelte:component") return `(e:CustomEvent<any>)=>void`;
return nativeEventHandlerType;
}
const attrName = `on:${eventName}`;
const importSvelteHTMLElements =
"import('svelte/elements').SvelteHTMLElements";
return [
`'${elementName}' extends infer EL`,
/**/ `?(`,
/**/ /**/ `EL extends keyof ${importSvelteHTMLElements}`,
/**/ /**/ `?(`,
/**/ /**/ /**/ `'${attrName}' extends infer ATTR`,
/**/ /**/ /**/ `?(`,
/**/ /**/ /**/ /**/ `ATTR extends keyof ${importSvelteHTMLElements}[EL]`,
/**/ /**/ /**/ /**/ /**/ `?${importSvelteHTMLElements}[EL][ATTR]`,
/**/ /**/ /**/ /**/ /**/ `:${nativeEventHandlerType}`,
/**/ /**/ /**/ `)`,
/**/ /**/ /**/ `:never`,
/**/ /**/ `)`,
/**/ /**/ `:${nativeEventHandlerType}`,
/**/ `)`,
/**/ `:never`,
].join("");
const svelteHTMLElementsType = "import('svelte/elements').SvelteHTMLElements";
return conditional({
check: `'${elementName}'`,
extends: "infer EL",
true: conditional({
check: `EL`,
extends: `keyof ${svelteHTMLElementsType}`,
true: conditional({
check: `'${attrName}'`,
extends: "infer ATTR",
true: conditional({
check: `ATTR`,
extends: `keyof ${svelteHTMLElementsType}[EL]`,
true: `${svelteHTMLElementsType}[EL][ATTR]`,
false: nativeEventHandlerType,
}),
false: `never`,
}),
false: nativeEventHandlerType,
}),
false: `never`,
});

/** Generate `C extends E ? T : F` type. */
function conditional(types: {
check: string;
extends: string;
true: string;
false: string;
}) {
return `${types.check} extends ${types.extends}?(${types.true}):(${types.false})`;
}
}

/** Convert for Class Directive */
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
/* eslint eslint-comments/require-description: 0, @typescript-eslint/explicit-module-boundary-types: 0 */
import { BASIC_PARSER_OPTIONS } from "../../../src/parser/test-utils";
import { generateParserOptions } from "../../../src/parser/test-utils";
import * as ts from "@typescript-eslint/parser";

export function getConfig() {
return {
parser: "svelte-eslint-parser",
parserOptions: {
...BASIC_PARSER_OPTIONS,
parser: { ts },
},
parserOptions: generateParserOptions({ parser: { ts } }),
env: {
browser: true,
es2021: true,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
/* eslint eslint-comments/require-description: 0, @typescript-eslint/explicit-module-boundary-types: 0 */
import { BASIC_PARSER_OPTIONS } from "../../../src/parser/test-utils";
import { generateParserOptions } from "../../../src/parser/test-utils";
import * as parser from "@typescript-eslint/parser";

export function getConfig() {
return {
parser: "svelte-eslint-parser",
parserOptions: {
...BASIC_PARSER_OPTIONS,
parser,
},
parserOptions: generateParserOptions({ parser }),
env: {
browser: true,
es2021: true,
Expand Down
4 changes: 2 additions & 2 deletions tests/fixtures/integrations/type-info-tests/await-setup.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint eslint-comments/require-description: 0, @typescript-eslint/explicit-module-boundary-types: 0 */
import type { Linter } from "eslint";
import { BASIC_PARSER_OPTIONS } from "../../../src/parser/test-utils";
import { generateParserOptions } from "../../../src/parser/test-utils";
import { rules } from "@typescript-eslint/eslint-plugin";
export function setupLinter(linter: Linter) {
linter.defineRule(
Expand All @@ -12,7 +12,7 @@ export function setupLinter(linter: Linter) {
export function getConfig() {
return {
parser: "svelte-eslint-parser",
parserOptions: BASIC_PARSER_OPTIONS,
parserOptions: generateParserOptions(),
rules: {
"@typescript-eslint/no-unsafe-call": "error",
},
Expand Down
4 changes: 2 additions & 2 deletions tests/fixtures/integrations/type-info-tests/i18n-setup.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint eslint-comments/require-description: 0, @typescript-eslint/explicit-module-boundary-types: 0 */
import type { Linter } from "eslint";
import { BASIC_PARSER_OPTIONS } from "../../../src/parser/test-utils";
import { generateParserOptions } from "../../../src/parser/test-utils";
import { rules } from "@typescript-eslint/eslint-plugin";
export function setupLinter(linter: Linter) {
linter.defineRule(
Expand All @@ -12,7 +12,7 @@ export function setupLinter(linter: Linter) {
export function getConfig() {
return {
parser: "svelte-eslint-parser",
parserOptions: BASIC_PARSER_OPTIONS,
parserOptions: generateParserOptions(),
rules: {
"@typescript-eslint/no-unsafe-call": "error",
},
Expand Down
4 changes: 2 additions & 2 deletions tests/fixtures/integrations/type-info-tests/issue226-setup.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint eslint-comments/require-description: 0, @typescript-eslint/explicit-module-boundary-types: 0 */
import type { Linter } from "eslint";
import { BASIC_PARSER_OPTIONS } from "../../../src/parser/test-utils";
import { generateParserOptions } from "../../../src/parser/test-utils";
import { rules } from "@typescript-eslint/eslint-plugin";
export function setupLinter(linter: Linter) {
linter.defineRule(
Expand All @@ -12,7 +12,7 @@ export function setupLinter(linter: Linter) {
export function getConfig() {
return {
parser: "svelte-eslint-parser",
parserOptions: BASIC_PARSER_OPTIONS,
parserOptions: generateParserOptions(),
rules: {
"@typescript-eslint/no-unsafe-argument": "error",
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint eslint-comments/require-description: 0, @typescript-eslint/explicit-module-boundary-types: 0 */
import type { Linter } from "eslint";
import { BASIC_PARSER_OPTIONS } from "../../../src/parser/test-utils";
import { generateParserOptions } from "../../../src/parser/test-utils";
import { rules } from "@typescript-eslint/eslint-plugin";
export function setupLinter(linter: Linter) {
linter.defineRule(
Expand All @@ -12,7 +12,7 @@ export function setupLinter(linter: Linter) {
export function getConfig() {
return {
parser: "svelte-eslint-parser",
parserOptions: BASIC_PARSER_OPTIONS,
parserOptions: generateParserOptions(),
rules: {
"@typescript-eslint/no-unnecessary-condition": "error",
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint eslint-comments/require-description: 0, @typescript-eslint/explicit-module-boundary-types: 0 */
import type { Linter } from "eslint";
import { BASIC_PARSER_OPTIONS } from "../../../src/parser/test-utils";
import { generateParserOptions } from "../../../src/parser/test-utils";
import { rules } from "@typescript-eslint/eslint-plugin";
export function setupLinter(linter: Linter) {
linter.defineRule(
Expand All @@ -12,7 +12,7 @@ export function setupLinter(linter: Linter) {
export function getConfig() {
return {
parser: "svelte-eslint-parser",
parserOptions: BASIC_PARSER_OPTIONS,
parserOptions: generateParserOptions(),
rules: {
"@typescript-eslint/no-unnecessary-condition": "error",
},
Expand Down
4 changes: 2 additions & 2 deletions tests/fixtures/integrations/type-info-tests/reactive-setup.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint eslint-comments/require-description: 0, @typescript-eslint/explicit-module-boundary-types: 0 */
import type { Linter } from "eslint";
import { BASIC_PARSER_OPTIONS } from "../../../src/parser/test-utils";
import { generateParserOptions } from "../../../src/parser/test-utils";
import { rules } from "@typescript-eslint/eslint-plugin";
export function setupLinter(linter: Linter) {
linter.defineRule(
Expand Down Expand Up @@ -28,7 +28,7 @@ export function setupLinter(linter: Linter) {
export function getConfig() {
return {
parser: "svelte-eslint-parser",
parserOptions: BASIC_PARSER_OPTIONS,
parserOptions: generateParserOptions(),
rules: {
"@typescript-eslint/no-unsafe-argument": "error",
"@typescript-eslint/no-unsafe-assignment": "error",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint eslint-comments/require-description: 0, @typescript-eslint/explicit-module-boundary-types: 0 */
import type { Linter } from "eslint";
import { BASIC_PARSER_OPTIONS } from "../../../src/parser/test-utils";
import { generateParserOptions } from "../../../src/parser/test-utils";
import { rules } from "@typescript-eslint/eslint-plugin";
export function setupLinter(linter: Linter) {
linter.defineRule(
Expand All @@ -16,7 +16,7 @@ export function setupLinter(linter: Linter) {
export function getConfig() {
return {
parser: "svelte-eslint-parser",
parserOptions: BASIC_PARSER_OPTIONS,
parserOptions: generateParserOptions(),
rules: {
"@typescript-eslint/no-unsafe-assignment": "error",
"@typescript-eslint/no-unsafe-member-access": "error",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint eslint-comments/require-description: 0, @typescript-eslint/explicit-module-boundary-types: 0 */
import type { Linter } from "eslint";
import { BASIC_PARSER_OPTIONS } from "../../../src/parser/test-utils";
import { generateParserOptions } from "../../../src/parser/test-utils";
import { rules } from "@typescript-eslint/eslint-plugin";
export function setupLinter(linter: Linter) {
linter.defineRule(
Expand All @@ -16,7 +16,7 @@ export function setupLinter(linter: Linter) {
export function getConfig() {
return {
parser: "svelte-eslint-parser",
parserOptions: BASIC_PARSER_OPTIONS,
parserOptions: generateParserOptions(),
rules: {
"@typescript-eslint/no-confusing-void-expression": "error",
"@typescript-eslint/explicit-function-return-type": "error",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint eslint-comments/require-description: 0, @typescript-eslint/explicit-module-boundary-types: 0 */
import type { Linter } from "eslint";
import { BASIC_PARSER_OPTIONS } from "../../../src/parser/test-utils";
import { generateParserOptions } from "../../../src/parser/test-utils";
import { rules } from "@typescript-eslint/eslint-plugin";
export function setupLinter(linter: Linter) {
linter.defineRule(
Expand All @@ -12,7 +12,7 @@ export function setupLinter(linter: Linter) {
export function getConfig() {
return {
parser: "svelte-eslint-parser",
parserOptions: BASIC_PARSER_OPTIONS,
parserOptions: generateParserOptions(),
rules: {
"@typescript-eslint/no-misused-promises": "error",
},
Expand Down
5 changes: 4 additions & 1 deletion tests/fixtures/parser/ast/ts-event05-input.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
</script>

<Component on:foo="{e=>{
// TODO: e.detail is number
// e.detail is number
// `@typescript-eslint/parser` doesn't get the correct types.
// Using `typescript-eslint-parser-for-extra-files` will give we the correct types.
// See `ts-event06-input.svelte` test case
e.detail;
}}" />
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
{
"ruleId": "no-unused-expressions",
"code": "e.detail;",
"line": 7,
"line": 10,
"column": 5
}
]

0 comments on commit 96a72a5

Please sign in to comment.