Skip to content

fix(language-core): avoid generic type loss due to destructured props #4821

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

Merged
merged 8 commits into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 9 additions & 12 deletions packages/language-core/lib/codegen/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,10 @@ export function collectVars(
ts: typeof import('typescript'),
node: ts.Node,
ast: ts.SourceFile,
results: string[] = [],
includesRest = true
results: string[] = []
) {
const identifiers = collectIdentifiers(ts, node, [], includesRest);
for (const id of identifiers) {
const identifiers = collectIdentifiers(ts, node, []);
for (const [id] of identifiers) {
results.push(getNodeText(ts, id, ast));
}
return results;
Expand All @@ -58,28 +57,26 @@ export function collectVars(
export function collectIdentifiers(
ts: typeof import('typescript'),
node: ts.Node,
results: ts.Identifier[] = [],
includesRest = true
results: [id: ts.Identifier, isRest: boolean][] = [],
isRest = false
) {
if (ts.isIdentifier(node)) {
results.push(node);
results.push([node, isRest]);
}
else if (ts.isObjectBindingPattern(node)) {
for (const el of node.elements) {
if (includesRest || !el.dotDotDotToken) {
collectIdentifiers(ts, el.name, results, includesRest);
}
collectIdentifiers(ts, el.name, results, !!el.dotDotDotToken);
}
}
else if (ts.isArrayBindingPattern(node)) {
for (const el of node.elements) {
if (ts.isBindingElement(el)) {
collectIdentifiers(ts, el.name, results, includesRest);
collectIdentifiers(ts, el.name, results, !!el.dotDotDotToken);
}
}
}
else {
ts.forEachChild(node, node => collectIdentifiers(ts, node, results, includesRest));
ts.forEachChild(node, node => collectIdentifiers(ts, node, results, false));
}
return results;
}
Expand Down
1 change: 1 addition & 0 deletions packages/language-core/lib/codegen/script/template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ function* generateCssVars(options: ScriptCodegenOptions, ctx: TemplateCodegenCon
for (const [segment, offset, onlyError] of forEachInterpolationSegment(
options.ts,
undefined,
undefined,
ctx,
cssBind.text,
cssBind.offset,
Expand Down
1 change: 1 addition & 0 deletions packages/language-core/lib/codegen/template/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export interface TemplateCodegenOptions {
edited: boolean;
scriptSetupBindingNames: Set<string>;
scriptSetupImportComponentNames: Set<string>;
destructuredPropNames: Set<string>;
templateRefNames: Set<string>;
hasDefineSlots?: boolean;
slotsAssignName?: string;
Expand Down
15 changes: 12 additions & 3 deletions packages/language-core/lib/codegen/template/interpolation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export function* generateInterpolation(
}[] = [];
for (let [section, offset, onlyError] of forEachInterpolationSegment(
options.ts,
options.destructuredPropNames,
options.templateRefNames,
ctx,
code,
Expand Down Expand Up @@ -72,6 +73,7 @@ export function* generateInterpolation(

export function* forEachInterpolationSegment(
ts: typeof import('typescript'),
destructuredPropNames: Set<string> | undefined,
templateRefNames: Set<string> | undefined,
ctx: TemplateCodegenContext,
code: string,
Expand Down Expand Up @@ -101,6 +103,9 @@ export function* forEachInterpolationSegment(
isShorthand: isShorthand,
offset: getStartEnd(ts, id, ast).start,
});
if (destructuredPropNames?.has(text)) {
return;
}
if (offset !== undefined) {
ctx.accessExternalVariable(text, offset + getStartEnd(ts, id, ast).start);
}
Expand All @@ -127,7 +132,7 @@ export function* forEachInterpolationSegment(
const curVar = ctxVars[i];
const nextVar = ctxVars[i + 1];

yield* generateVar(code, templateRefNames, curVar, nextVar);
yield* generateVar(code, destructuredPropNames, templateRefNames, curVar, nextVar);

if (nextVar.isShorthand) {
yield [code.substring(curVar.offset + curVar.text.length, nextVar.offset + nextVar.text.length), curVar.offset + curVar.text.length];
Expand All @@ -139,7 +144,7 @@ export function* forEachInterpolationSegment(
}

const lastVar = ctxVars.at(-1)!;
yield* generateVar(code, templateRefNames, lastVar);
yield* generateVar(code, destructuredPropNames, templateRefNames, lastVar);
yield [code.substring(lastVar.offset + lastVar.text.length), lastVar.offset + lastVar.text.length];
}
else {
Expand All @@ -149,6 +154,7 @@ export function* forEachInterpolationSegment(

function* generateVar(
code: string,
destructuredPropNames: Set<string> | undefined,
templateRefNames: Set<string> | undefined,
curVar: {
text: string,
Expand All @@ -165,14 +171,17 @@ function* generateVar(
// fix https://github.com/vuejs/language-tools/issues/1264
yield ['', nextVar.offset, true];

const isDestructuredProp = destructuredPropNames?.has(curVar.text) ?? false;
const isTemplateRef = templateRefNames?.has(curVar.text) ?? false;
if (isTemplateRef) {
yield [`__VLS_unref(`, undefined];
yield [code.substring(curVar.offset, curVar.offset + curVar.text.length), curVar.offset];
yield [`)`, undefined];
}
else {
yield [`__VLS_ctx.`, undefined];
if (!isDestructuredProp) {
yield [`__VLS_ctx.`, undefined];
}
yield [code.substring(curVar.offset, curVar.offset + curVar.text.length), curVar.offset];
}
}
Expand Down
17 changes: 14 additions & 3 deletions packages/language-core/lib/parsers/scriptSetupRanges.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type * as ts from 'typescript';
import type { VueCompilerOptions, TextRange } from '../types';
import { collectVars } from '../codegen/common';
import { collectIdentifiers } from '../codegen/common';

export interface ScriptSetupRanges extends ReturnType<typeof parseScriptSetupRanges> { }

Expand All @@ -15,7 +15,8 @@ export function parseScriptSetupRanges(

const props: {
name?: string;
destructured?: string[];
destructured?: Set<string>;
destructuredRest?: string;
define?: ReturnType<typeof parseDefineFunction> & {
statement: TextRange;
};
Expand Down Expand Up @@ -308,7 +309,17 @@ export function parseScriptSetupRanges(
else if (vueCompilerOptions.macros.defineProps.includes(callText)) {
if (ts.isVariableDeclaration(parent)) {
if (ts.isObjectBindingPattern(parent.name)) {
props.destructured = collectVars(ts, parent.name, ast, [], false);
props.destructured = new Set();
const identifiers = collectIdentifiers(ts, parent.name, []);
for (const [id, isRest] of identifiers) {
const name = getNodeText(ts, id, ast);
if (isRest) {
props.destructuredRest = name;
}
else {
props.destructured.add(name);
}
}
}
else {
props.name = getNodeText(ts, parent.name, ast);
Expand Down
12 changes: 12 additions & 0 deletions packages/language-core/lib/plugins/vue-tsx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ function createTsx(
edited: ctx.vueCompilerOptions.__test || (fileEditTimes.get(fileName) ?? 0) >= 2,
scriptSetupBindingNames: scriptSetupBindingNames(),
scriptSetupImportComponentNames: scriptSetupImportComponentNames(),
destructuredPropNames: destructuredPropNames(),
templateRefNames: templateRefNames(),
hasDefineSlots: hasDefineSlots(),
slotsAssignName: slotsAssignName(),
Expand Down Expand Up @@ -144,6 +145,17 @@ function createTsx(
}
return newNames;
});
const destructuredPropNames = computed<Set<string>>(oldNames => {
const newNames = scriptSetupRanges()?.props.destructured ?? new Set();
const rest = scriptSetupRanges()?.props.destructuredRest;
if (rest) {
newNames.add(rest);
}
if (oldNames && twoSetsEqual(newNames, oldNames)) {
return oldNames;
}
return newNames;
});
const templateRefNames = computed<Set<string>>(oldNames => {
const newNames = new Set(
scriptSetupRanges()?.templateRefs
Expand Down
6 changes: 3 additions & 3 deletions packages/language-service/lib/plugins/vue-inlayhints.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ type Scope = Record<string, boolean>;
export function findDestructuredProps(
ts: typeof import('typescript'),
ast: ts.SourceFile,
props: string[]
props: Set<string>
) {
const rootScope: Scope = {};
const scopeStack: Scope[] = [rootScope];
Expand Down Expand Up @@ -180,7 +180,7 @@ export function findDestructuredProps(
&& ts.isCallExpression(initializer)
&& initializer.expression.getText(ast) === 'defineProps';

for (const id of collectIdentifiers(ts, name)) {
for (const [id] of collectIdentifiers(ts, name)) {
if (isDefineProps) {
excludedIds.add(id);
} else {
Expand All @@ -196,7 +196,7 @@ export function findDestructuredProps(
}

for (const p of parameters) {
for (const id of collectIdentifiers(ts, p)) {
for (const [id] of collectIdentifiers(ts, p)) {
registerLocalBinding(id);
}
}
Expand Down
1 change: 1 addition & 0 deletions test-workspace/tsc/passedFixtures/vue2/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"../vue3/#4646",
"../vue3/#4649",
"../vue3/#4777",
"../vue3/#4820",
"../vue3/components",
"../vue3/defineEmits",
"../vue3/defineModel",
Expand Down
1 change: 1 addition & 0 deletions test-workspace/tsc/passedFixtures/vue3.4/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
"exclude": [
"../vue3/#3820",
"../vue3/#4777",
"../vue3/#4820",
"../vue3/rootEl",
"../vue3/templateRef",
"../vue3/templateRef_native",
Expand Down
22 changes: 22 additions & 0 deletions test-workspace/tsc/passedFixtures/vue3/#4820/main.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<script lang="ts">
type Foo = {
a: string;
b: number;
};

declare function func<TFoo extends Foo>(foo: TFoo, fooKey: keyof TFoo): any;
</script>

<script setup lang="ts" generic="TFoo extends Foo, TFooKey extends keyof TFoo">
const { foo, fooKey, ...props } = defineProps<{
foo: TFoo;
fooKey: TFooKey;
bar: TFoo;
barKey: TFooKey;
}>();
</script>

<template>
{{ func(foo, fooKey) }}
{{ func(props.bar, props.barKey) }}
</template>