Skip to content

Commit ad623c3

Browse files
dummdidummdummdidumm
authored
(fix) mark props as optional in ts when no typings given (#370)
Before this, `export let a = ''` would not be marked as optional because it had no type. #347 Also fixes a bug where file type was incorrectly inferred as jsx which occurs when user uses the default languages feature of svelte-preprocess. Instead of computing the file type inside svelte2tsx, it is now passed as an option.
1 parent c0a21b4 commit ad623c3

File tree

26 files changed

+125
-103
lines changed

26 files changed

+125
-103
lines changed

‎packages/language-server/src/plugins/typescript/DocumentSnapshot.ts

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,19 @@ export namespace DocumentSnapshot {
8282
* @param options options that apply to the svelte document
8383
*/
8484
export function fromDocument(document: Document, options: SvelteSnapshotOptions) {
85-
const { tsxMap, text, parserError, nrPrependedLines } = preprocessSvelteFile(
85+
const { tsxMap, text, parserError, nrPrependedLines, scriptKind } = preprocessSvelteFile(
8686
document,
8787
options,
8888
);
8989

90-
return new SvelteDocumentSnapshot(document, parserError, text, nrPrependedLines, tsxMap);
90+
return new SvelteDocumentSnapshot(
91+
document,
92+
parserError,
93+
scriptKind,
94+
text,
95+
nrPrependedLines,
96+
tsxMap,
97+
);
9198
}
9299

93100
/**
@@ -115,10 +122,18 @@ function preprocessSvelteFile(document: Document, options: SvelteSnapshotOptions
115122
let nrPrependedLines = 0;
116123
let text = document.getText();
117124

125+
const scriptKind = [
126+
getScriptKindFromAttributes(document.scriptInfo?.attributes ?? {}),
127+
getScriptKindFromAttributes(document.moduleScriptInfo?.attributes ?? {}),
128+
].includes(ts.ScriptKind.TSX)
129+
? ts.ScriptKind.TSX
130+
: ts.ScriptKind.JSX;
131+
118132
try {
119133
const tsx = svelte2tsx(text, {
120134
strictMode: options.strictMode,
121135
filename: document.getFilePath() ?? undefined,
136+
isTsFile: scriptKind === ts.ScriptKind.TSX,
122137
});
123138
text = tsx.code;
124139
tsxMap = tsx.map;
@@ -149,21 +164,21 @@ function preprocessSvelteFile(document: Document, options: SvelteSnapshotOptions
149164
text = document.scriptInfo ? document.scriptInfo.content : '';
150165
}
151166

152-
return { tsxMap, text, parserError, nrPrependedLines };
167+
return { tsxMap, text, parserError, nrPrependedLines, scriptKind };
153168
}
154169

155170
/**
156171
* A svelte document snapshot suitable for the ts language service and the plugin.
157172
*/
158173
export class SvelteDocumentSnapshot implements DocumentSnapshot {
159174
private fragment?: SvelteSnapshotFragment;
160-
private _scriptKind?: ts.ScriptKind;
161175

162176
version = this.parent.version;
163177

164178
constructor(
165179
private readonly parent: Document,
166180
public readonly parserError: ParserError | null,
181+
public readonly scriptKind: ts.ScriptKind,
167182
private readonly text: string,
168183
private readonly nrPrependedLines: number,
169184
private readonly tsxMap?: RawSourceMap,
@@ -173,21 +188,6 @@ export class SvelteDocumentSnapshot implements DocumentSnapshot {
173188
return this.parent.getFilePath() || '';
174189
}
175190

176-
get scriptKind() {
177-
if (!this._scriptKind) {
178-
const scriptKind = getScriptKindFromAttributes(
179-
this.parent.scriptInfo?.attributes ?? {},
180-
);
181-
const moduleScriptKind = getScriptKindFromAttributes(
182-
this.parent.moduleScriptInfo?.attributes ?? {},
183-
);
184-
this._scriptKind = [scriptKind, moduleScriptKind].includes(ts.ScriptKind.TSX)
185-
? ts.ScriptKind.TSX
186-
: ts.ScriptKind.JSX;
187-
}
188-
return this._scriptKind;
189-
}
190-
191191
getText(start: number, end: number) {
192192
return this.text.substring(start, end);
193193
}

‎packages/svelte2tsx/index.d.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,22 @@ type SvelteCompiledToTsx = {
55

66
export default function svelte2tsx(
77
svelte: string,
8-
options?: { filename?: string; strictMode?: boolean }
8+
options?: {
9+
/**
10+
* Path of the file
11+
*/
12+
filename?: string;
13+
/**
14+
* Whether or not TS strictMode is enabled
15+
*/
16+
strictMode?: boolean;
17+
/**
18+
* If the given file uses TypeScript inside script.
19+
* This cannot be inferred from `svelte2tsx` by looking
20+
* at the attributes of the script tag because the
21+
* user may have set a default-language through
22+
* `svelte-preprocess`.
23+
*/
24+
isTsFile?: boolean;
25+
}
926
): SvelteCompiledToTsx

‎packages/svelte2tsx/src/interfaces.ts

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,6 @@
11
import MagicString from 'magic-string';
22
import { Node } from 'estree-walker';
3-
4-
export type ExportedNames = Map<
5-
string,
6-
{
7-
type?: string;
8-
identifierText?: string;
9-
required?: boolean;
10-
}
11-
>;
3+
import { ExportedNames } from './nodes/ExportedNames';
124

135
export interface InstanceScriptProcessResult {
146
exportedNames: ExportedNames;
@@ -22,4 +14,5 @@ export interface CreateRenderFunctionPara extends InstanceScriptProcessResult {
2214
scriptTag: Node;
2315
scriptDestination: number;
2416
slots: Map<string, Map<string, string>>;
17+
isTsFile: boolean;
2518
}

‎packages/svelte2tsx/src/nodes/ExportedNames.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
export class ExportedNames extends Map<
2+
string,
3+
{
4+
type?: string;
5+
identifierText?: string;
6+
required?: boolean;
7+
}
8+
> {
9+
/**
10+
* Creates a string from the collected props
11+
*
12+
* @param isTsFile Whether this is a TypeScript file or not.
13+
*/
14+
createPropsStr(isTsFile: boolean) {
15+
const names = Array.from(this.entries());
16+
17+
const returnElements = names.map(([key, value]) => {
18+
// Important to not use shorthand props for rename functionality
19+
return `${value.identifierText || key}: ${key}`;
20+
});
21+
22+
if (
23+
!isTsFile ||
24+
names.length === 0 ||
25+
names.every(([_, value]) => !value.type && value.required)
26+
) {
27+
// No exports or only `typeof` exports -> omit the `as {...}` completely.
28+
// If not TS, omit the types to not have a "cannot use types in jsx" error.
29+
return `{${returnElements.join(' , ')}}`;
30+
}
31+
32+
const returnElementsType = names.map(([key, value]) => {
33+
const identifier = `${value.identifierText || key}${value.required ? '' : '?'}`;
34+
if (!value.type) {
35+
return `${identifier}: typeof ${key}`;
36+
}
37+
38+
return `${identifier}: ${value.type}`;
39+
});
40+
41+
return `{${returnElements.join(' , ')}} as {${returnElementsType.join(', ')}}`;
42+
}
43+
}

‎packages/svelte2tsx/src/svelte2tsx.ts

Lines changed: 16 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@ import { convertHtmlxToJsx } from './htmlxtojsx';
77
import { Node } from 'estree-walker';
88
import * as ts from 'typescript';
99
import { findExortKeyword } from './utils/tsAst';
10-
import { ExportedNames, InstanceScriptProcessResult, CreateRenderFunctionPara } from './interfaces';
10+
import { InstanceScriptProcessResult, CreateRenderFunctionPara } from './interfaces';
1111
import { createRenderFunctionGetterStr, createClassGetters } from './nodes/exportgetters';
12+
import { ExportedNames } from './nodes/ExportedNames';
1213

1314
function AttributeValueAsJsExpression(htmlx: string, attr: Node): string {
1415
if (attr.value.length == 0) return "''"; //wut?
@@ -367,7 +368,6 @@ function processSvelteTemplate(str: MagicString): TemplateProcessResult {
367368
};
368369
}
369370

370-
371371
function processInstanceScriptContent(str: MagicString, script: Node): InstanceScriptProcessResult {
372372
const htmlx = str.original;
373373
const scriptContent = htmlx.substring(script.content.start, script.content.end);
@@ -379,7 +379,7 @@ function processInstanceScriptContent(str: MagicString, script: Node): InstanceS
379379
ts.ScriptKind.TS,
380380
);
381381
const astOffset = script.content.start;
382-
const exportedNames: ExportedNames = new Map();
382+
const exportedNames = new ExportedNames();
383383
const getters = new Set<string>();
384384

385385
const implicitTopLevelNames: Map<string, number> = new Map();
@@ -614,11 +614,7 @@ function processInstanceScriptContent(str: MagicString, script: Node): InstanceS
614614
ts.forEachChild(list, (node) => {
615615
if (ts.isVariableDeclaration(node)) {
616616
if (ts.isIdentifier(node.name)) {
617-
if (node.type) {
618-
addExport(node.name, node.name, node.type, !node.initializer);
619-
} else {
620-
addExport(node.name, null, null, !node.initializer);
621-
}
617+
addExport(node.name, node.name, node.type, !node.initializer);
622618
} else if (
623619
ts.isObjectBindingPattern(node.name) ||
624620
ts.isArrayBindingPattern(node.name)
@@ -796,7 +792,7 @@ function processInstanceScriptContent(str: MagicString, script: Node): InstanceS
796792
for (const [name, pos] of implicitTopLevelNames.entries()) {
797793
if (!rootScope.declared.has(name)) {
798794
// remove '$:' label
799-
str.remove(pos + astOffset, pos + astOffset+2);
795+
str.remove(pos + astOffset, pos + astOffset + 2);
800796
str.prependRight(pos + astOffset, `let `);
801797
}
802798
}
@@ -879,29 +875,6 @@ export function classNameFromFilename(filename: string): string | undefined {
879875
}
880876
}
881877

882-
function isTsFile(scriptTag: Node | undefined, moduleScriptTag: Node | undefined) {
883-
return tagIsLangTs(scriptTag) || tagIsLangTs(moduleScriptTag);
884-
885-
function tagIsLangTs(tag: Node | undefined) {
886-
return tag?.attributes?.some((attr) => {
887-
if (attr.name !== 'lang' && attr.name !== 'type') {
888-
return false;
889-
}
890-
891-
const type = attr.value[0]?.raw;
892-
switch (type) {
893-
case 'ts':
894-
case 'typescript':
895-
case 'text/ts':
896-
case 'text/typescript':
897-
return true;
898-
default:
899-
return false;
900-
}
901-
});
902-
}
903-
}
904-
905878
function processModuleScriptTag(str: MagicString, script: Node) {
906879
const htmlx = str.original;
907880

@@ -919,8 +892,9 @@ function createRenderFunction({
919892
slots,
920893
getters,
921894
exportedNames,
895+
isTsFile,
922896
uses$$props,
923-
uses$$restProps
897+
uses$$restProps,
924898
}: CreateRenderFunctionPara) {
925899
const htmlx = str.original;
926900
let propsDecl = '';
@@ -958,40 +932,16 @@ function createRenderFunction({
958932
.join(', ') +
959933
'}';
960934

961-
const returnString = `\nreturn { props: ${createPropsStr(
962-
exportedNames,
935+
const returnString = `\nreturn { props: ${exportedNames.createPropsStr(
936+
isTsFile,
963937
)}, slots: ${slotsAsDef}, getters: ${createRenderFunctionGetterStr(getters)} }}`;
964938
str.append(returnString);
965939
}
966940

967-
function createPropsStr(exportedNames: ExportedNames) {
968-
const names = Array.from(exportedNames.entries());
969-
970-
const returnElements = names.map(([key, value]) => {
971-
// Important to not use shorthand props for rename functionality
972-
return `${value.identifierText || key}: ${key}`;
973-
});
974-
975-
if (names.length === 0 || !names.some(([_, value]) => !!value.type)) {
976-
// No exports or only `typeof` exports -> omit the `as {...}` completely
977-
// -> 2nd case could be that it's because it's a js file without typing, so
978-
// omit the types to not have a "cannot use types in jsx" error
979-
return `{${returnElements.join(' , ')}}`;
980-
}
981-
982-
const returnElementsType = names.map(([key, value]) => {
983-
const identifier = value.identifierText || key;
984-
if (!value.type) {
985-
return `${identifier}: typeof ${key}`;
986-
}
987-
988-
return `${identifier}${value.required ? '' : '?'}: ${value.type}`;
989-
});
990-
991-
return `{${returnElements.join(' , ')}} as {${returnElementsType.join(', ')}}`;
992-
}
993-
994-
export function svelte2tsx(svelte: string, options?: { filename?: string; strictMode?: boolean }) {
941+
export function svelte2tsx(
942+
svelte: string,
943+
options?: { filename?: string; strictMode?: boolean; isTsFile?: boolean },
944+
) {
995945
const str = new MagicString(svelte);
996946
// process the htmlx as a svelte template
997947
let {
@@ -1021,7 +971,7 @@ export function svelte2tsx(svelte: string, options?: { filename?: string; strict
1021971
}
1022972

1023973
//move the instance script and process the content
1024-
let exportedNames: ExportedNames = new Map();
974+
let exportedNames = new ExportedNames();
1025975
let getters = new Set<string>();
1026976
if (scriptTag) {
1027977
//ensure it is between the module script and the rest of the template (the variables need to be declared before the jsx template)
@@ -1043,6 +993,7 @@ export function svelte2tsx(svelte: string, options?: { filename?: string; strict
1043993
slots,
1044994
getters,
1045995
exportedNames,
996+
isTsFile: options?.isTsFile,
1046997
uses$$props,
1047998
uses$$restProps,
1048999
});
@@ -1058,7 +1009,7 @@ export function svelte2tsx(svelte: string, options?: { filename?: string; strict
10581009
str,
10591010
uses$$props || uses$$restProps,
10601011
!!options?.strictMode,
1061-
isTsFile(scriptTag, moduleScriptTag),
1012+
options?.isTsFile,
10621013
getters,
10631014
className,
10641015
componentDocumentation,

‎packages/svelte2tsx/test/svelte2tsx/index.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,11 @@ describe('svelte2tsx', () => {
1919
const input = fs.readFileSync(`${__dirname}/samples/${dir}/input.svelte`, 'utf-8').replace(/\s+$/, '').replace(/\r\n/g, "\n");
2020
const expectedOutput = fs.readFileSync(`${__dirname}/samples/${dir}/expected.tsx`, 'utf-8').replace(/\s+$/, '').replace(/\r\n/g, "\n");
2121

22-
const { map, code} = svelte2tsx(input, {strictMode: dir.endsWith('strictMode'), filename: 'input.svelte'});
22+
const { map, code} = svelte2tsx(input, {
23+
strictMode: dir.includes('strictMode'),
24+
isTsFile: dir.startsWith('ts-'),
25+
filename: 'input.svelte'
26+
});
2327
assert.equal(code, expectedOutput);
2428
});
2529
});

‎packages/svelte2tsx/test/svelte2tsx/samples/export-js-strictMode/expected.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
<></>;function render() {
22

3-
let a: number;
4-
let b: number | undefined;
5-
let c: number = 123;c = __sveltets_any(c);;
3+
let a;
4+
let b;
5+
let c = 123;
66
;
77
<></>
8-
return { props: {a: a , b: b , c: c} as {a: number, b: number | undefined, c?: number}, slots: {}, getters: {} }}
8+
return { props: {a: a , b: b , c: c}, slots: {}, getters: {} }}
99

1010
export default class Input__SvelteComponent_ {
1111
$$prop_def = __sveltets_partial(render().props)

‎packages/svelte2tsx/test/svelte2tsx/samples/export-js-strictMode/input.svelte

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<script>
2-
export let a: number;
3-
export let b: number | undefined;
4-
export let c: number = 123;
2+
export let a;
3+
export let b;
4+
export let c = 123;
55
</script>

‎packages/svelte2tsx/test/svelte2tsx/samples/export-arrow-function/expected.tsx renamed to ‎packages/svelte2tsx/test/svelte2tsx/samples/ts-export-arrow-function/expected.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
}
77
;
88
<></>
9-
return { props: {f: f}, slots: {}, getters: {} }}
9+
return { props: {f: f} as {f?: typeof f}, slots: {}, getters: {} }}
1010

1111
export default class Input__SvelteComponent_ {
1212
$$prop_def = __sveltets_partial(render().props)

‎packages/svelte2tsx/test/svelte2tsx/samples/export-arrow-function/input.svelte renamed to ‎packages/svelte2tsx/test/svelte2tsx/samples/ts-export-arrow-function/input.svelte

File renamed without changes.

‎packages/svelte2tsx/test/svelte2tsx/samples/export-const/expected.tsx renamed to ‎packages/svelte2tsx/test/svelte2tsx/samples/ts-export-const/expected.tsx

File renamed without changes.

‎packages/svelte2tsx/test/svelte2tsx/samples/export-const/input.svelte renamed to ‎packages/svelte2tsx/test/svelte2tsx/samples/ts-export-const/input.svelte

File renamed without changes.

‎packages/svelte2tsx/test/svelte2tsx/samples/ts-export-has-initializer/expected.tsx

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<></>;function render() {
2+
3+
let a = '';
4+
;
5+
<></>
6+
return { props: {a: a} as {a?: typeof a}, slots: {}, getters: {} }}
7+
8+
export default class Input__SvelteComponent_ {
9+
$$prop_def = __sveltets_partial(render().props)
10+
$$slot_def = render().slots
11+
}

0 commit comments

Comments
 (0)