Skip to content

Commit

Permalink
fix: various snippet fixes (#2449)
Browse files Browse the repository at this point in the history
#2444 #2417 #2425 #2385 (comment)

The snippet is now transformed into a function with the return type so that The default parameter type can be inferred:

`const foo = (a: string, b = 1): ReturnType<import('svelte').Snippet> => {}`
  • Loading branch information
jasonlyu123 committed Jul 30, 2024
1 parent 4a9ef5e commit 0f7bd88
Show file tree
Hide file tree
Showing 25 changed files with 368 additions and 90 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ import {
convertToLocationForReferenceOrDefinition,
convertToLocationRange,
isInScript,
isSvelte2tsxShimFile,
isSvelteFilePath,
symbolKindFromString
} from './utils';
Expand Down Expand Up @@ -387,7 +388,7 @@ export class TypeScriptPlugin

const result = await Promise.all(
defs.definitions.map(async (def) => {
if (def.fileName.endsWith('svelte-shims.d.ts')) {
if (isSvelte2tsxShimFile(def.fileName)) {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ import {
isInGeneratedCode,
findChildOfKind,
findRenderFunction,
findClosestContainingNode,
SnapshotMap
SnapshotMap,
startsWithIgnoredPosition
} from './utils';
import { convertRange } from '../utils';
import { convertRange, isSvelte2tsxShimFile } from '../utils';

export class InlayHintProviderImpl implements InlayHintProvider {
constructor(private readonly lsAndTsDocResolver: LSAndTSDocResolver) {}
Expand Down Expand Up @@ -195,10 +195,19 @@ export class InlayHintProviderImpl implements InlayHintProvider {
return false;
}

const node = findClosestContainingNode(
if (inlayHint.displayParts?.some((v) => isSvelte2tsxShimFile(v.file))) {
return true;
}

const hasParameterWithSamePosition = (node: ts.CallExpression | ts.NewExpression) =>
node.arguments !== undefined &&
node.arguments.some((arg) => arg.getStart() === inlayHint.position);

const node = findContainingNode(
sourceFile,
{ start: inlayHint.position, length: 0 },
ts.isCallOrNewExpression
(node): node is ts.CallExpression | ts.NewExpression =>
ts.isCallOrNewExpression(node) && hasParameterWithSamePosition(node)
);

if (!node) {
Expand All @@ -224,6 +233,10 @@ export class InlayHintProviderImpl implements InlayHintProvider {
return false;
}

if (startsWithIgnoredPosition(sourceFile.text, inlayHint.position)) {
return true;
}

const declaration = findContainingNode(
sourceFile,
{ start: inlayHint.position, length: 0 },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ export function isComponentAtPosition(

export const IGNORE_START_COMMENT = '/*Ωignore_startΩ*/';
export const IGNORE_END_COMMENT = '/*Ωignore_endΩ*/';
export const IGNORE_POSITION_COMMENT = '/*Ωignore_positionΩ*/';

/**
* Surrounds given string with a start/end comment which marks it
Expand All @@ -105,6 +106,10 @@ export function isInGeneratedCode(text: string, start: number, end: number = sta
return (lastStart > lastEnd || lastEnd === nextEnd) && lastStart < nextEnd;
}

export function startsWithIgnoredPosition(text: string, offset: number) {
return text.slice(offset).startsWith(IGNORE_POSITION_COMMENT);
}

/**
* Checks if this is a text span that is inside svelte2tsx-generated code
* (has no mapping to the original)
Expand Down
4 changes: 4 additions & 0 deletions packages/language-server/src/plugins/typescript/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -371,3 +371,7 @@ export function hasTsExtensions(fileName: string) {
fileName.endsWith(ts.Extension.Ts)
);
}

export function isSvelte2tsxShimFile(fileName: string | undefined) {
return fileName?.endsWith('svelte-shims.d.ts') || fileName?.endsWith('svelte-shims-v4.d.ts');
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script lang="ts">
let { foo }: {
foo: import('svelte').Snippet<[a: '']>
children?: import('svelte').Snippet
required: import('svelte').Snippet
} = $props();
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
[
{
"range": { "start": { "line": 4, "character": 1 }, "end": { "line": 4, "character": 14 } },
"severity": 1,
"source": "ts",
"message": "Property 'required' is missing in type '{ children: () => any; foo: (this: void, a: \"\") => any; }' but required in type '$$ComponentProps'.",
"code": 2741,
"tags": []
},
{
"range": { "start": { "line": 6, "character": 9 }, "end": { "line": 6, "character": 18 } },
"severity": 1,
"source": "ts",
"message": "This comparison appears to be unintentional because the types '\"\"' and '\"b\"' have no overlap.",
"code": 2367,
"tags": []
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<script lang="ts">
import SnippetParent from "./SnippetParent.svelte";
</script>

<SnippetParent>
{#snippet foo(a)}
{a === 'b'}
{/snippet}

{@render foo('')}
</SnippetParent>
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
[
{
"range": {
"start": { "line": 10, "character": 9 },
"end": { "line": 10, "character": 18 }
},
"severity": 1,
"source": "js",
"message": "This comparison appears to be unintentional because the types 'number' and 'string' have no overlap.",
"code": 2367,
"tags": []
},
{
"range": {
"start": { "line": 16, "character": 12 },
"end": { "line": 16, "character": 15 }
},
"severity": 1,
"source": "js",
"message": "Argument of type '\"c\"' is not assignable to parameter of type 'TypeA'.",
"code": 2345,
"tags": []
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<script>
// @ts-check
/**
* @typedef {'a' | 'b'} TypeA
*/
</script>

<!--no error-->
{#snippet hi(/**@type {TypeA}*/a, b = 2)}
{a}
{#if b === 'a'}
{b}
{/if}
{/snippet}

<!--error-->
{@render hi('c', 'd')}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[
{
"range": {
"start": { "line": 15, "character": 9 },
"end": { "line": 15, "character": 16 }
},
"severity": 1,
"source": "ts",
"message": "Cannot find name 'nested1'.",
"code": 2304,
"tags": []
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<script lang="ts">
// no error
top
</script>

<!--no error-->
<div>
<!-- {@render nested1()} -->
{#snippet nested1()}{/snippet}
{@render nested1()}
</div>

{#snippet top()}{/snippet}

<!--error-->
{@render nested1()}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
[
{
"label": [
{
"value": "a",
"location": {
"range": {
"start": { "line": 0, "character": 14 },
"end": { "line": 0, "character": 15 }
},
"uri": "<workspaceUri>/snippet.v5/input.svelte"
}
},
{ "value": ":" }
],
"position": { "line": 4, "character": 13 },
"kind": 2,
"paddingRight": true
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{#snippet hi2(a = 1)}
hello world
{/snippet}

{@render hi2(1)}
10 changes: 9 additions & 1 deletion packages/language-server/test/plugins/typescript/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ import { FileMap } from '../../../src/lib/documents/fileCollection';
import { LSConfigManager } from '../../../src/ls-config';
import { LSAndTSDocResolver } from '../../../src/plugins';
import { createGetCanonicalFileName, normalizePath, pathToUrl } from '../../../src/utils';
import { VERSION } from 'svelte/compiler';

const isSvelte5Plus = Number(VERSION.split('.')[0]) >= 5;

export function createVirtualTsSystem(currentDirectory: string): ts.System {
const virtualFs = new FileMap<string>();
Expand Down Expand Up @@ -198,7 +201,12 @@ export function createSnapshotTester<
}

if (existsSync(inputFile)) {
const _it = dir.endsWith('.only') ? it.only : it;
const _it =
dir.endsWith('.v5') && !isSvelte5Plus
? it.skip
: dir.endsWith('.only')
? it.only
: it;
_it(dir.substring(__dirname.length), () => executeTest(inputFile, testOptions));
} else {
const _describe = dir.endsWith('.only') ? describe.only : describe;
Expand Down
10 changes: 9 additions & 1 deletion packages/svelte2tsx/src/htmlxtojsx_v2/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { handleSpread } from './nodes/Spread';
import { handleStyleDirective } from './nodes/StyleDirective';
import { handleText } from './nodes/Text';
import { handleTransitionDirective } from './nodes/Transition';
import { handleImplicitChildren, handleSnippet } from './nodes/SnippetBlock';
import { handleImplicitChildren, handleSnippet, hoistSnippetBlock } from './nodes/SnippetBlock';
import { handleRenderTag } from './nodes/RenderTag';

type Walker = (node: TemplateNode, parent: BaseNode, prop: string, index: number) => void;
Expand Down Expand Up @@ -63,6 +63,8 @@ export function convertHtmlxToJsx(
const rootSnippets: Array<[number, number]> = [];
let element: Element | InlineComponent | undefined;

const pendingSnippetHoistCheck = new Set<BaseNode>();

walk(ast as any, {
enter: (estreeTypedNode, estreeTypedParent, prop: string, index: number) => {
const node = estreeTypedNode as TemplateNode;
Expand Down Expand Up @@ -94,6 +96,8 @@ export function convertHtmlxToJsx(
if (parent === ast) {
// root snippet -> move to instance script
rootSnippets.push([node.start, node.end]);
} else {
pendingSnippetHoistCheck.add(parent);
}
break;
case 'MustacheTag':
Expand Down Expand Up @@ -251,6 +255,10 @@ export function convertHtmlxToJsx(
}
});

for (const node of pendingSnippetHoistCheck) {
hoistSnippetBlock(str, node);
}

return rootSnippets;
}

Expand Down
16 changes: 16 additions & 0 deletions packages/svelte2tsx/src/htmlxtojsx_v2/nodes/InlineComponent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export class InlineComponent {
private propsTransformation: TransformationArray = [];
private eventsTransformation: TransformationArray = [];
private slotLetsTransformation?: [TransformationArray, TransformationArray];
private snippetPropsTransformation: TransformationArray = [];
private endTransformation: TransformationArray = [];
private startTagStart: number;
private startTagEnd: number;
Expand Down Expand Up @@ -159,6 +160,12 @@ export class InlineComponent {
this.slotLetsTransformation[1].push(...transformation, ',');
}

addImplicitSnippetProp(name: [number, number], transforms: TransformationArray): void {
this.addProp([name], transforms);

this.snippetPropsTransformation.push(this.str.original.slice(name[0], name[1]));
}

/**
* Add something right after the start tag end.
*/
Expand Down Expand Up @@ -191,6 +198,13 @@ export class InlineComponent {
this.endTransformation.push('}');
}

const snippetPropVariables = this.snippetPropsTransformation?.join(', ');
const snippetPropVariablesDeclaration = snippetPropVariables
? surroundWithIgnoreComments(
`const {${snippetPropVariables}} = ${this.name}.$$prop_def;`
)
: '';

if (this.isSelfclosing) {
this.endTransformation.push('}');
transform(this.str, this.startTagStart, this.startTagEnd, this.startTagEnd, [
Expand All @@ -203,6 +217,7 @@ export class InlineComponent {
...this.startEndTransformation,
...this.eventsTransformation,
...defaultSlotLetTransformation,
snippetPropVariablesDeclaration,
...this.endTransformation
]);
} else {
Expand All @@ -223,6 +238,7 @@ export class InlineComponent {
...this.propsTransformation,
...this.startEndTransformation,
...this.eventsTransformation,
snippetPropVariablesDeclaration,
...defaultSlotLetTransformation
]);
transform(this.str, endStart, this.node.end, this.node.end, this.endTransformation);
Expand Down
Loading

0 comments on commit 0f7bd88

Please sign in to comment.