From edfb295cf008004a291b45cec29265c05c713ca6 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 26 Feb 2021 11:49:18 +0100 Subject: [PATCH 01/17] (fix) if conditions control flow This adds an IfScope to svelte2tsx which keeps track of the if scope including nested ifs. That scope is used to prepend the resulting condition check to all inner lambda functions that svelte2tsx needs to create for slot, each, await. This way, TypeScript's control flow can determine the type of checked variables and no longer loses that info due to the lambda functions. #619 --- .../features/DiagnosticsProvider.test.ts | 133 ++++++++++++++++++ ...iagnostics-if-control-flow-imported.svelte | 5 + .../diagnostics-if-control-flow.svelte | 54 +++++++ packages/svelte2tsx/src/htmlxtojsx/index.ts | 20 ++- .../svelte2tsx/src/htmlxtojsx/nodes/await.ts | 18 ++- .../src/htmlxtojsx/nodes/component.ts | 15 +- .../svelte2tsx/src/htmlxtojsx/nodes/each.ts | 14 +- .../src/htmlxtojsx/nodes/if-else.ts | 110 ++++++++++++++- .../if-nested-await-block/expected.jsx | 24 ++++ .../if-nested-await-block/input.svelte | 24 ++++ .../samples/if-nested-each-block/expected.jsx | 20 +++ .../samples/if-nested-each-block/input.svelte | 20 +++ .../samples/if-nested-slot-let/expected.jsx | 22 +++ .../samples/if-nested-slot-let/input.svelte | 22 +++ .../expected.tsx | 2 +- 15 files changed, 479 insertions(+), 24 deletions(-) create mode 100644 packages/language-server/test/plugins/typescript/testfiles/diagnostics/diagnostics-if-control-flow-imported.svelte create mode 100644 packages/language-server/test/plugins/typescript/testfiles/diagnostics/diagnostics-if-control-flow.svelte create mode 100644 packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/expected.jsx create mode 100644 packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/input.svelte create mode 100644 packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block/expected.jsx create mode 100644 packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block/input.svelte create mode 100644 packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let/expected.jsx create mode 100644 packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let/input.svelte diff --git a/packages/language-server/test/plugins/typescript/features/DiagnosticsProvider.test.ts b/packages/language-server/test/plugins/typescript/features/DiagnosticsProvider.test.ts index f8427917d..2475d8258 100644 --- a/packages/language-server/test/plugins/typescript/features/DiagnosticsProvider.test.ts +++ b/packages/language-server/test/plugins/typescript/features/DiagnosticsProvider.test.ts @@ -477,4 +477,137 @@ describe('DiagnosticsProvider', () => { } ]); }); + + it('if control flow', async () => { + const { plugin, document } = setup('diagnostics-if-control-flow.svelte'); + const diagnostics = await plugin.getDiagnostics(document); + + assert.deepStrictEqual(diagnostics, [ + { + code: 2367, + message: + "This condition will always return 'false' since the types 'string' and 'boolean' have no overlap.", + range: { + end: { + character: 15, + line: 13 + }, + start: { + character: 5, + line: 13 + } + }, + severity: 1, + source: 'ts', + tags: [] + }, + { + code: 2367, + message: + "This condition will always return 'false' since the types 'string' and 'boolean' have no overlap.", + range: { + end: { + character: 19, + line: 16 + }, + start: { + character: 9, + line: 16 + } + }, + severity: 1, + source: 'ts', + tags: [] + }, + { + code: 2367, + message: + "This condition will always return 'false' since the types 'string' and 'boolean' have no overlap.", + range: { + end: { + character: 19, + line: 20 + }, + start: { + character: 9, + line: 20 + } + }, + severity: 1, + source: 'ts', + tags: [] + }, + { + code: 2532, + message: "Object is possibly 'undefined'.", + range: { + end: { + character: 14, + line: 33 + }, + start: { + character: 13, + line: 33 + } + }, + severity: 1, + source: 'ts', + tags: [] + }, + { + code: 2367, + message: + "This condition will always return 'false' since the types 'boolean' and 'string' have no overlap.", + range: { + end: { + character: 26, + line: 35 + }, + start: { + character: 17, + line: 35 + } + }, + severity: 1, + source: 'ts', + tags: [] + }, + { + code: 2367, + message: + "This condition will always return 'false' since the types 'string' and 'boolean' have no overlap.", + range: { + end: { + character: 25, + line: 44 + }, + start: { + character: 13, + line: 44 + } + }, + severity: 1, + source: 'ts', + tags: [] + }, + { + code: 2322, + message: + "Type 'string | boolean' is not assignable to type 'string'.\n Type 'boolean' is not assignable to type 'string'.", + range: { + end: { + character: 8, + line: 53 + }, + start: { + character: 1, + line: 53 + } + }, + severity: 1, + source: 'ts', + tags: [] + } + ]); + }); }); diff --git a/packages/language-server/test/plugins/typescript/testfiles/diagnostics/diagnostics-if-control-flow-imported.svelte b/packages/language-server/test/plugins/typescript/testfiles/diagnostics/diagnostics-if-control-flow-imported.svelte new file mode 100644 index 000000000..794223e78 --- /dev/null +++ b/packages/language-server/test/plugins/typescript/testfiles/diagnostics/diagnostics-if-control-flow-imported.svelte @@ -0,0 +1,5 @@ + + + diff --git a/packages/language-server/test/plugins/typescript/testfiles/diagnostics/diagnostics-if-control-flow.svelte b/packages/language-server/test/plugins/typescript/testfiles/diagnostics/diagnostics-if-control-flow.svelte new file mode 100644 index 000000000..d4c19b844 --- /dev/null +++ b/packages/language-server/test/plugins/typescript/testfiles/diagnostics/diagnostics-if-control-flow.svelte @@ -0,0 +1,54 @@ + + +{#if typeof a === 'string'} + {a === true} + {assignA = a} + {#each [] as foo} + {a === true} + {assignA = a} + {foo} + {:else} + {a === true} + {assignA = a} + {/each} + {#if b} + {#await aPromise} + {b.a} + {:then x} + {b.a === x} + {/await} + {b.a} + {:else} + {b === true} + + {b.a === foo} + {#if typeof foo === 'boolean'} + {foo === a} + {:else} + {foo === a} + {/if} + + {/if} +{:else} + {#if typeof $store === 'string'} + {#each [] as foo} + {$store === a} + {foo} + {/each} + {:else} + {$store === a} + {/if} +{/if} + +{a === true} +{assignA = a} diff --git a/packages/svelte2tsx/src/htmlxtojsx/index.ts b/packages/svelte2tsx/src/htmlxtojsx/index.ts index 059c903d6..ae041351d 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/index.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/index.ts @@ -15,7 +15,7 @@ import { handleDebug } from './nodes/debug'; import { handleEach } from './nodes/each'; import { handleElement } from './nodes/element'; import { handleEventHandler } from './nodes/event-handler'; -import { handleElse, handleIf } from './nodes/if-else'; +import { handleElse, handleIf, IfScope } from './nodes/if-else'; import { handleRawHtml } from './nodes/raw-html'; import { handleSvelteTag } from './nodes/svelte-tag'; import { handleTransitionDirective } from './nodes/transition-directive'; @@ -46,21 +46,24 @@ export function convertHtmlxToJsx( str.prepend('<>'); str.append(''); + let ifScope = new IfScope(); + (svelte as any).walk(ast, { enter: (node: Node, parent: Node, prop: string, index: number) => { try { switch (node.type) { case 'IfBlock': - handleIf(htmlx, str, node); + handleIf(htmlx, str, node, ifScope); + ifScope = ifScope.getChild(); break; case 'EachBlock': - handleEach(htmlx, str, node); + handleEach(htmlx, str, node, ifScope); break; case 'ElseBlock': - handleElse(htmlx, str, node, parent); + handleElse(htmlx, str, node, parent, ifScope); break; case 'AwaitBlock': - handleAwait(htmlx, str, node); + handleAwait(htmlx, str, node, ifScope); break; case 'KeyBlock': handleKey(htmlx, str, node); @@ -72,7 +75,7 @@ export function convertHtmlxToJsx( handleDebug(htmlx, str, node); break; case 'InlineComponent': - handleComponent(htmlx, str, node); + handleComponent(htmlx, str, node, ifScope); break; case 'Element': handleElement(htmlx, str, node); @@ -128,6 +131,11 @@ export function convertHtmlxToJsx( leave: (node: Node, parent: Node, prop: string, index: number) => { try { + switch (node.type) { + case 'IfBlock': + ifScope = ifScope.getParent(); + break; + } if (onLeave) { onLeave(node, parent, prop, index); } diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/await.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/await.ts index ec4cb784f..15d658846 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/await.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/await.ts @@ -1,16 +1,22 @@ import MagicString from 'magic-string'; import { Node } from 'estree-walker'; +import { IfScope } from './if-else'; /** * Transform {#await ...} into something JSX understands */ -export function handleAwait(htmlx: string, str: MagicString, awaitBlock: Node): void { +export function handleAwait( + htmlx: string, + str: MagicString, + awaitBlock: Node, + ifScope: IfScope +): void { // {#await somePromise then value} -> // {() => {let _$$p = (somePromise); str.overwrite(awaitBlock.start, awaitBlock.expression.start, '{() => {let _$$p = ('); // then value } | {:then value} | {await ..} .. {/await} -> - // __sveltets_awaitThen(_$$p, (value) => {<> + // __sveltets_awaitThen(_$$p, (value) => {(possibleIfCondition && )<> let thenStart: number; let thenEnd: number; if (!awaitBlock.then.skip) { @@ -30,7 +36,7 @@ export function handleAwait(htmlx: string, str: MagicString, awaitBlock: Node): // somePromise} -> somePromise); str.overwrite(awaitBlock.expression.end, awaitEnd + 1, ');'); - str.appendRight(awaitEnd + 1, ' <>'); + str.appendRight(awaitEnd + 1, ` ${ifScope.addPossibleIfCondition()}<>`); } else { // {await ... then ...} thenStart = htmlx.indexOf('then', awaitBlock.expression.end); @@ -53,9 +59,9 @@ export function handleAwait(htmlx: string, str: MagicString, awaitBlock: Node): if (awaitBlock.value) { str.overwrite(thenStart, awaitBlock.value.start, '__sveltets_awaitThen(_$$p, ('); - str.overwrite(awaitBlock.value.end, thenEnd, ') => {<>'); + str.overwrite(awaitBlock.value.end, thenEnd, `) => {${ifScope.addPossibleIfCondition()}<>`); } else { - const awaitThenFn = '__sveltets_awaitThen(_$$p, () => {<>'; + const awaitThenFn = `__sveltets_awaitThen(_$$p, () => {${ifScope.addPossibleIfCondition()}<>`; if (thenStart === thenEnd) { str.appendLeft(thenStart, awaitThenFn); } else { @@ -74,7 +80,7 @@ export function handleAwait(htmlx: string, str: MagicString, awaitBlock: Node): const errorEnd = awaitBlock.error ? awaitBlock.error.end : errorStart; const catchEnd = htmlx.indexOf('}', errorEnd) + 1; str.overwrite(catchStart, errorStart, '}, ('); - str.overwrite(errorEnd, catchEnd, ') => {<>'); + str.overwrite(errorEnd, catchEnd, `) => {${ifScope.addPossibleIfCondition()}<>`); } // {/await} -> // <>})} diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/component.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/component.ts index 5c8fc4803..36506d049 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/component.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/component.ts @@ -3,11 +3,12 @@ import { Node } from 'estree-walker'; import { getSlotName } from '../../utils/svelteAst'; import { beforeStart } from '../utils/node-utils'; import { getSingleSlotDef } from '../../svelte2tsx/nodes/slot'; +import { IfScope } from './if-else'; /** * Handle `` and slot-specific transformations. */ -export function handleComponent(htmlx: string, str: MagicString, el: Node): void { +export function handleComponent(htmlx: string, str: MagicString, el: Node, ifScope: IfScope): void { //we need to remove : if it is a svelte component if (el.name.startsWith('svelte:')) { const colon = htmlx.indexOf(':', el.start); @@ -21,7 +22,7 @@ export function handleComponent(htmlx: string, str: MagicString, el: Node): void } //we only need to do something if there is a let or slot - handleSlot(htmlx, str, el, el, 'default'); + handleSlot(htmlx, str, el, el, 'default', ifScope); //walk the direct children looking for slots. We do this here because we need the name of our component for handleSlot //we could lean on leave/enter, but I am lazy @@ -29,7 +30,7 @@ export function handleComponent(htmlx: string, str: MagicString, el: Node): void for (const child of el.children) { const slotName = getSlotName(child); if (slotName) { - handleSlot(htmlx, str, child, el, slotName); + handleSlot(htmlx, str, child, el, slotName, ifScope); } } } @@ -39,7 +40,8 @@ function handleSlot( str: MagicString, slotEl: Node, component: Node, - slotName: string + slotName: string, + ifScope: IfScope ): void { //collect "let" definitions const slotElIsComponent = slotEl === component; @@ -83,7 +85,10 @@ function handleSlot( return; } str.appendLeft(slotDefInsertionPoint, '{() => { let {'); - str.appendRight(slotDefInsertionPoint, `} = ${getSingleSlotDef(component, slotName)}` + ';<>'); + str.appendRight( + slotDefInsertionPoint, + `} = ${getSingleSlotDef(component, slotName)}` + `;${ifScope.addPossibleIfCondition()}<>` + ); const closeSlotDefInsertionPoint = slotElIsComponent ? htmlx.lastIndexOf('<', slotEl.end - 1) diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/each.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/each.ts index 6b8ef3040..5fdf35c10 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/each.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/each.ts @@ -1,12 +1,18 @@ import MagicString from 'magic-string'; import { Node } from 'estree-walker'; +import { IfScope } from './if-else'; /** * Transform each block into something JSX can understand. */ -export function handleEach(htmlx: string, str: MagicString, eachBlock: Node): void { +export function handleEach( + htmlx: string, + str: MagicString, + eachBlock: Node, + ifScope: IfScope +): void { // {#each items as item,i (key)} -> - // {__sveltets_each(items, (item,i) => (key) && <> + // {__sveltets_each(items, (item,i) => (key) && (possible if expression &&) <> str.overwrite(eachBlock.start, eachBlock.expression.start, '{__sveltets_each('); str.overwrite(eachBlock.expression.end, eachBlock.context.start, ', ('); @@ -24,10 +30,10 @@ export function handleEach(htmlx: string, str: MagicString, eachBlock: Node): vo str.prependLeft(contextEnd, ') =>'); if (eachBlock.key) { const endEachStart = htmlx.indexOf('}', eachBlock.key.end); - str.overwrite(endEachStart, endEachStart + 1, ' && <>'); + str.overwrite(endEachStart, endEachStart + 1, ` && ${ifScope.addPossibleIfCondition()}<>`); } else { const endEachStart = htmlx.indexOf('}', contextEnd); - str.overwrite(endEachStart, endEachStart + 1, ' <>'); + str.overwrite(endEachStart, endEachStart + 1, ` ${ifScope.addPossibleIfCondition()}<>`); } const endEach = htmlx.lastIndexOf('{', eachBlock.end - 1); // {/each} -> )} or {:else} -> )} diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/if-else.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-else.ts index 59021a397..189e6105c 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/if-else.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-else.ts @@ -1,10 +1,104 @@ import MagicString from 'magic-string'; import { Node } from 'estree-walker'; +enum IfType { + If, + ElseIf, + Else +} + +interface IfCondition { + type: IfType.If; + condition: string; +} + +interface ElseIfCondition { + type: IfType.ElseIf; + condition: string; + parent: IfCondition | ElseIfCondition; +} + +interface ElseCondition { + type: IfType.Else; + parent: IfCondition | ElseIfCondition; +} + +type Condition = IfCondition | ElseIfCondition | ElseCondition; + +function getFullCondition(condition: Condition): string { + switch (condition.type) { + case IfType.If: + return _getFullCondition(condition, false); + case IfType.ElseIf: + return _getFullCondition(condition, false); + case IfType.Else: + return _getFullCondition(condition, false); + } +} + +function _getFullCondition(condition: Condition, negate: boolean): string { + switch (condition.type) { + case IfType.If: + return negate ? `!(${condition.condition})` : `(${condition.condition})`; + case IfType.ElseIf: + return `${_getFullCondition(condition.parent, true)} && ${negate ? '!' : ''}(${ + condition.condition + })`; + case IfType.Else: + return `${_getFullCondition(condition.parent, true)}`; + } +} + +export class IfScope { + private child?: IfScope; + + constructor(private current?: Condition, private parent?: IfScope) {} + + getFullCondition(): string { + if (!this.current) { + return ''; + } + + const parentCondition = this.parent?.getFullCondition(); + const condition = `(${getFullCondition(this.current)})`; + return parentCondition ? `(${parentCondition}) && ${condition}` : condition; + } + + addPossibleIfCondition(): string { + const condition = this.getFullCondition(); + return condition ? `${condition} && ` : ''; + } + + addNestedIf(condition: string): void { + const ifScope = new IfScope({ condition, type: IfType.If }, this); + this.child = ifScope; + } + + addElseIf(condition: string): void { + this.current = { + condition, + parent: this.current as IfCondition | ElseIfCondition, + type: IfType.ElseIf + }; + } + + addElse(): void { + this.current = { parent: this.current as IfCondition | ElseIfCondition, type: IfType.Else }; + } + + getChild(): IfScope { + return this.child || this; + } + + getParent(): IfScope { + return this.parent || this; + } +} + /** * {# if ...}...{/if} ---> {() => {if(...){<>...}}} */ -export function handleIf(htmlx: string, str: MagicString, ifBlock: Node): void { +export function handleIf(htmlx: string, str: MagicString, ifBlock: Node, ifScope: IfScope): void { const endIf = htmlx.lastIndexOf('{', ifBlock.end - 1); if (ifBlock.elseif) { @@ -14,6 +108,8 @@ export function handleIf(htmlx: string, str: MagicString, ifBlock: Node): void { str.overwrite(elseIfStart, ifBlock.expression.start, ' : (', { contentOnly: true }); str.overwrite(ifBlock.expression.end, elseIfConditionEnd, ') ? <>'); + ifScope.addElseIf(str.original.substring(ifBlock.expression.start, ifBlock.expression.end)); + if (!ifBlock.else) { str.appendLeft(endIf, ' : <>'); } @@ -25,6 +121,8 @@ export function handleIf(htmlx: string, str: MagicString, ifBlock: Node): void { const end = htmlx.indexOf('}', ifBlock.expression.end); str.overwrite(ifBlock.expression.end, end + 1, ') ? <>', { contentOnly: true }); + ifScope.addNestedIf(str.original.substring(ifBlock.expression.start, ifBlock.expression.end)); + if (ifBlock.else) { // {/if} -> } str.overwrite(endIf, ifBlock.end, ' }', { contentOnly: true }); @@ -37,7 +135,13 @@ export function handleIf(htmlx: string, str: MagicString, ifBlock: Node): void { /** * {:else} ---> : <> */ -export function handleElse(htmlx: string, str: MagicString, elseBlock: Node, parent: Node): void { +export function handleElse( + htmlx: string, + str: MagicString, + elseBlock: Node, + parent: Node, + ifScope: IfScope +): void { if ( parent.type !== 'IfBlock' || (elseBlock.children[0]?.type === 'IfBlock' && elseBlock.children[0]?.elseif) @@ -48,4 +152,6 @@ export function handleElse(htmlx: string, str: MagicString, elseBlock: Node, par const elseword = htmlx.lastIndexOf(':else', elseEnd); const elseStart = htmlx.lastIndexOf('{', elseword); str.overwrite(elseStart, elseEnd + 1, ' : <>'); + + ifScope.addElse(); } diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/expected.jsx b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/expected.jsx new file mode 100644 index 000000000..8955e9127 --- /dev/null +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/expected.jsx @@ -0,0 +1,24 @@ +<>{(hello) ? <> + {() => {let _$$p = (x); __sveltets_awaitThen(_$$p, (y) => {((hello)) && <> + {y} + })}} + {(hi && bye) ? <> + {() => {let _$$p = (x); __sveltets_awaitThen(_$$p, (y) => {(((hello))) && ((hi && bye)) && <> + {y} + }, () => {(((hello))) && ((hi && bye)) && <> + z + })}} + : (cool) ? <> + {() => {let _$$p = (x); (((hello))) && (!(hi && bye) && (cool)) && <> + loading + ; __sveltets_awaitThen(_$$p, (y) => {(((hello))) && (!(hi && bye) && (cool)) && <> + {y} + }, () => {(((hello))) && (!(hi && bye) && (cool)) && <> + z + })}} + : <> + {() => {let _$$p = (x); __sveltets_awaitThen(_$$p, (y) => {(((hello))) && (!(hi && bye) && !(cool)) && <> + {y} + })}} + } + : <>} \ No newline at end of file diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/input.svelte b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/input.svelte new file mode 100644 index 000000000..1075a1446 --- /dev/null +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/input.svelte @@ -0,0 +1,24 @@ +{#if hello} + {#await x then y} + {y} + {/await} + {#if hi && bye} + {#await x then y} + {y} + {:catch} + z + {/await} + {:else if cool} + {#await x} + loading + {:then y} + {y} + {:catch} + z + {/await} + {:else} + {#await x then y} + {y} + {/await} + {/if} +{/if} diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block/expected.jsx b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block/expected.jsx new file mode 100644 index 000000000..4b531db5b --- /dev/null +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block/expected.jsx @@ -0,0 +1,20 @@ +<>{(hello) ? <> + {__sveltets_each(items, (item,i) => (item.id) && ((hello)) && <> +
{item}{i}
+ )} + {(hi && bye) ? <> + {__sveltets_each(items, (item) => (((hello))) && ((hi && bye)) && <> +
{item}
+ )} +

hi

+ + : (cool) ? <> + {__sveltets_each(items, (item,i) => (((hello))) && (!(hi && bye) && (cool)) && <> +
{item}{i}
+ )} + : <> + {__sveltets_each(items, (item) => (((hello))) && (!(hi && bye) && !(cool)) && <> +
{item}
+ )} + } + : <>} \ No newline at end of file diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block/input.svelte b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block/input.svelte new file mode 100644 index 000000000..f30d8bc4d --- /dev/null +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block/input.svelte @@ -0,0 +1,20 @@ +{#if hello} + {#each items as item,i (item.id)} +
{item}{i}
+ {/each} + {#if hi && bye} + {#each items as item} +
{item}
+ {:else} +

hi

+ {/each} + {:else if cool} + {#each items as item,i} +
{item}{i}
+ {/each} + {:else} + {#each items as item} +
{item}
+ {/each} + {/if} +{/if} diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let/expected.jsx b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let/expected.jsx new file mode 100644 index 000000000..527498a80 --- /dev/null +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let/expected.jsx @@ -0,0 +1,22 @@ +<>{(hello) ? <> + {() => { let {foo} = __sveltets_instanceOf(Comp).$$slot_def['default'];((hello)) && <> + {foo} + }} + {(hi && bye) ? <> + {() => { let {foo:bar} = __sveltets_instanceOf(Comp).$$slot_def['default'];(((hello))) && ((hi && bye)) && <> + {bar} + }} + : (cool) ? <> + + {() => { let {foo, foo1} = __sveltets_instanceOf(Comp).$$slot_def['named'];(((hello))) && (!(hi && bye) && (cool)) && <>
+ {foo} +
}} +
+ : <> + + {() => { let {foo:bar} = __sveltets_instanceOf(Comp).$$slot_def['named'];(((hello))) && (!(hi && bye) && !(cool)) && <>
+ {bar} +
}} +
+ } + : <>} \ No newline at end of file diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let/input.svelte b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let/input.svelte new file mode 100644 index 000000000..7d2188ed8 --- /dev/null +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let/input.svelte @@ -0,0 +1,22 @@ +{#if hello} + + {foo} + + {#if hi && bye} + + {bar} + + {:else if cool} + +
+ {foo} +
+
+ {:else} + +
+ {bar} +
+
+ {/if} +{/if} diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/uses-svelte-components-let-forward/expected.tsx b/packages/svelte2tsx/test/svelte2tsx/samples/uses-svelte-components-let-forward/expected.tsx index d3a14e4f1..3262e56c0 100644 --- a/packages/svelte2tsx/test/svelte2tsx/samples/uses-svelte-components-let-forward/expected.tsx +++ b/packages/svelte2tsx/test/svelte2tsx/samples/uses-svelte-components-let-forward/expected.tsx @@ -1,7 +1,7 @@ /// <>;function render() { <>{(true) ? <> -{() => { let {prop} = __sveltets_instanceOf(__sveltets_componentType()).$$slot_def['default'];<> +{() => { let {prop} = __sveltets_instanceOf(__sveltets_componentType()).$$slot_def['default'];((true)) && <> }} : <>} From 90251fc710e0dd8c2b7fada09f31aac5e9156485 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 1 Mar 2021 19:22:00 +0100 Subject: [PATCH 02/17] shadowed variables WIP --- packages/svelte2tsx/src/htmlxtojsx/index.ts | 71 ++++- .../svelte2tsx/src/htmlxtojsx/nodes/await.ts | 2 +- .../src/htmlxtojsx/nodes/component.ts | 42 +-- .../svelte2tsx/src/htmlxtojsx/nodes/each.ts | 12 +- .../src/htmlxtojsx/nodes/if-else.ts | 99 +------ .../src/htmlxtojsx/nodes/if-scope.ts | 279 ++++++++++++++++++ .../src/htmlxtojsx/utils/node-utils.ts | 33 ++- .../input.svelte | 35 +++ .../if-nested-await-block/expected.jsx | 10 +- .../if-nested-await-block/input.svelte | 10 +- .../expected.jsx | 20 ++ .../input.svelte | 20 ++ .../expected.jsx | 34 +++ .../input.svelte | 34 +++ 14 files changed, 568 insertions(+), 133 deletions(-) create mode 100644 packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts create mode 100644 packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/input.svelte create mode 100644 packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/expected.jsx create mode 100644 packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/input.svelte create mode 100644 packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed.solo/expected.jsx create mode 100644 packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed.solo/input.svelte diff --git a/packages/svelte2tsx/src/htmlxtojsx/index.ts b/packages/svelte2tsx/src/htmlxtojsx/index.ts index ae041351d..c8c8295d4 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/index.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/index.ts @@ -10,16 +10,21 @@ import { handleKey } from './nodes/key'; import { handleBinding } from './nodes/binding'; import { handleClassDirective } from './nodes/class-directive'; import { handleComment } from './nodes/comment'; -import { handleComponent } from './nodes/component'; +import { handleComponent, handleSlot, usesLet } from './nodes/component'; import { handleDebug } from './nodes/debug'; import { handleEach } from './nodes/each'; import { handleElement } from './nodes/element'; import { handleEventHandler } from './nodes/event-handler'; -import { handleElse, handleIf, IfScope } from './nodes/if-else'; +import { handleElse, handleIf } from './nodes/if-else'; +import { IfScope } from './nodes/if-scope'; import { handleRawHtml } from './nodes/raw-html'; import { handleSvelteTag } from './nodes/svelte-tag'; import { handleTransitionDirective } from './nodes/transition-directive'; import { handleText } from './nodes/text'; +import TemplateScope from '../svelte2tsx/nodes/TemplateScope'; +import { getSlotName, isDestructuringPatterns, isIdentifier } from '../utils/svelteAst'; +import { extract_identifiers } from 'periscopic'; +import { SvelteIdentifier } from '../interfaces'; type Walker = (node: Node, parent: Node, prop: string, index: number) => void; @@ -46,7 +51,27 @@ export function convertHtmlxToJsx( str.prepend('<>'); str.append(''); - let ifScope = new IfScope(); + const templateScope = { value: new TemplateScope() }; + const handleScopeEach = (node: Node) => { + templateScope.value = templateScope.value.child(); + if (node.context) { + handleScope(node.context, node, templateScope.value); + } + if (node.index) { + templateScope.value.add({ name: node.index, type: 'Identifier' }, node); + } + }; + const handleScopeAwait = (node: Node) => { + templateScope.value = templateScope.value.child(); + if (node.value) { + handleScope(node.value, node.then, templateScope.value); + } + if (node.error) { + handleScope(node.error, node.catch, templateScope.value); + } + }; + + let ifScope = new IfScope(templateScope); (svelte as any).walk(ast, { enter: (node: Node, parent: Node, prop: string, index: number) => { @@ -57,12 +82,14 @@ export function convertHtmlxToJsx( ifScope = ifScope.getChild(); break; case 'EachBlock': + handleScopeEach(node); handleEach(htmlx, str, node, ifScope); break; case 'ElseBlock': handleElse(htmlx, str, node, parent, ifScope); break; case 'AwaitBlock': + handleScopeAwait(node); handleAwait(htmlx, str, node, ifScope); break; case 'KeyBlock': @@ -75,9 +102,24 @@ export function convertHtmlxToJsx( handleDebug(htmlx, str, node); break; case 'InlineComponent': - handleComponent(htmlx, str, node, ifScope); + if (usesLet(node)) { + templateScope.value = templateScope.value.child(); + } + handleComponent(htmlx, str, node, ifScope, templateScope.value); break; case 'Element': + if (usesLet(node)) { + templateScope.value = templateScope.value.child(); + handleSlot( + htmlx, + str, + node, + parent, + getSlotName(node), + ifScope, + templateScope.value + ); + } handleElement(htmlx, str, node); break; case 'Comment': @@ -135,6 +177,16 @@ export function convertHtmlxToJsx( case 'IfBlock': ifScope = ifScope.getParent(); break; + case 'EachBlock': + case 'AwaitBlock': + templateScope.value = templateScope.value.parent; + break; + case 'InlineComponent': + case 'Element': + if (usesLet(node)) { + templateScope.value = templateScope.value.parent; + } + break; } if (onLeave) { onLeave(node, parent, prop, index); @@ -147,6 +199,17 @@ export function convertHtmlxToJsx( }); } +function handleScope(identifierDef: Node, owner: Node, templateScope: TemplateScope) { + if (isIdentifier(identifierDef)) { + templateScope.add(identifierDef, owner); + } + if (isDestructuringPatterns(identifierDef)) { + // the node object is returned as-it with no mutation + const identifiers = extract_identifiers(identifierDef) as SvelteIdentifier[]; + templateScope.addMany(identifiers, owner); + } +} + /** * @internal For testing only */ diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/await.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/await.ts index 15d658846..073ae78ba 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/await.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/await.ts @@ -1,6 +1,6 @@ import MagicString from 'magic-string'; import { Node } from 'estree-walker'; -import { IfScope } from './if-else'; +import { IfScope } from './if-scope'; /** * Transform {#await ...} into something JSX understands diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/component.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/component.ts index 36506d049..75ba40ce5 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/component.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/component.ts @@ -1,14 +1,20 @@ import MagicString from 'magic-string'; import { Node } from 'estree-walker'; -import { getSlotName } from '../../utils/svelteAst'; import { beforeStart } from '../utils/node-utils'; import { getSingleSlotDef } from '../../svelte2tsx/nodes/slot'; -import { IfScope } from './if-else'; +import { IfScope } from './if-scope'; +import TemplateScope from '../../svelte2tsx/nodes/TemplateScope'; /** * Handle `` and slot-specific transformations. */ -export function handleComponent(htmlx: string, str: MagicString, el: Node, ifScope: IfScope): void { +export function handleComponent( + htmlx: string, + str: MagicString, + el: Node, + ifScope: IfScope, + templateScope: TemplateScope +): void { //we need to remove : if it is a svelte component if (el.name.startsWith('svelte:')) { const colon = htmlx.indexOf(':', el.start); @@ -22,26 +28,21 @@ export function handleComponent(htmlx: string, str: MagicString, el: Node, ifSco } //we only need to do something if there is a let or slot - handleSlot(htmlx, str, el, el, 'default', ifScope); + handleSlot(htmlx, str, el, el, 'default', ifScope, templateScope); +} - //walk the direct children looking for slots. We do this here because we need the name of our component for handleSlot - //we could lean on leave/enter, but I am lazy - if (!el.children) return; - for (const child of el.children) { - const slotName = getSlotName(child); - if (slotName) { - handleSlot(htmlx, str, child, el, slotName, ifScope); - } - } +export function usesLet(node: Node) { + return node.attributes?.some((attr) => attr.type === 'Let'); } -function handleSlot( +export function handleSlot( htmlx: string, str: MagicString, slotEl: Node, component: Node, slotName: string, - ifScope: IfScope + ifScope: IfScope, + templateScope: TemplateScope ): void { //collect "let" definitions const slotElIsComponent = slotEl === component; @@ -72,6 +73,13 @@ function handleSlot( } else { str.remove(attr.start, attr.start + 'let:'.length); } + templateScope.add( + { + name: attr.expression?.name || attr.name, + type: 'Identifier' + }, + slotEl + ); hasMoved = true; if (attr.expression) { //overwrite the = as a : @@ -84,7 +92,7 @@ function handleSlot( if (!hasMoved) { return; } - str.appendLeft(slotDefInsertionPoint, '{() => { let {'); + str.appendLeft(slotDefInsertionPoint, `{${ifScope.addConstsPrefixIfNecessary()}() => { let {`); str.appendRight( slotDefInsertionPoint, `} = ${getSingleSlotDef(component, slotName)}` + `;${ifScope.addPossibleIfCondition()}<>` @@ -93,5 +101,5 @@ function handleSlot( const closeSlotDefInsertionPoint = slotElIsComponent ? htmlx.lastIndexOf('<', slotEl.end - 1) : slotEl.end; - str.appendLeft(closeSlotDefInsertionPoint, '}}'); + str.appendLeft(closeSlotDefInsertionPoint, `}}${ifScope.addConstsSuffixIfNecessary()}`); } diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/each.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/each.ts index 5fdf35c10..04581a01d 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/each.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/each.ts @@ -1,6 +1,6 @@ import MagicString from 'magic-string'; import { Node } from 'estree-walker'; -import { IfScope } from './if-else'; +import { IfScope } from './if-scope'; /** * Transform each block into something JSX can understand. @@ -13,7 +13,11 @@ export function handleEach( ): void { // {#each items as item,i (key)} -> // {__sveltets_each(items, (item,i) => (key) && (possible if expression &&) <> - str.overwrite(eachBlock.start, eachBlock.expression.start, '{__sveltets_each('); + str.overwrite( + eachBlock.start, + eachBlock.expression.start, + `{${ifScope.addConstsPrefixIfNecessary()}() => {__sveltets_each(` + ); str.overwrite(eachBlock.expression.end, eachBlock.context.start, ', ('); // {#each true, items as item} @@ -40,9 +44,9 @@ export function handleEach( if (eachBlock.else) { const elseEnd = htmlx.lastIndexOf('}', eachBlock.else.start); const elseStart = htmlx.lastIndexOf('{', elseEnd); - str.overwrite(elseStart, elseEnd + 1, ')}'); + str.overwrite(elseStart, elseEnd + 1, `)}}${ifScope.addConstsSuffixIfNecessary()}`); str.remove(endEach, eachBlock.end); } else { - str.overwrite(endEach, eachBlock.end, ')}'); + str.overwrite(endEach, eachBlock.end, `)}}${ifScope.addConstsSuffixIfNecessary()}`); } } diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/if-else.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-else.ts index 189e6105c..f59043285 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/if-else.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-else.ts @@ -1,99 +1,6 @@ import MagicString from 'magic-string'; import { Node } from 'estree-walker'; - -enum IfType { - If, - ElseIf, - Else -} - -interface IfCondition { - type: IfType.If; - condition: string; -} - -interface ElseIfCondition { - type: IfType.ElseIf; - condition: string; - parent: IfCondition | ElseIfCondition; -} - -interface ElseCondition { - type: IfType.Else; - parent: IfCondition | ElseIfCondition; -} - -type Condition = IfCondition | ElseIfCondition | ElseCondition; - -function getFullCondition(condition: Condition): string { - switch (condition.type) { - case IfType.If: - return _getFullCondition(condition, false); - case IfType.ElseIf: - return _getFullCondition(condition, false); - case IfType.Else: - return _getFullCondition(condition, false); - } -} - -function _getFullCondition(condition: Condition, negate: boolean): string { - switch (condition.type) { - case IfType.If: - return negate ? `!(${condition.condition})` : `(${condition.condition})`; - case IfType.ElseIf: - return `${_getFullCondition(condition.parent, true)} && ${negate ? '!' : ''}(${ - condition.condition - })`; - case IfType.Else: - return `${_getFullCondition(condition.parent, true)}`; - } -} - -export class IfScope { - private child?: IfScope; - - constructor(private current?: Condition, private parent?: IfScope) {} - - getFullCondition(): string { - if (!this.current) { - return ''; - } - - const parentCondition = this.parent?.getFullCondition(); - const condition = `(${getFullCondition(this.current)})`; - return parentCondition ? `(${parentCondition}) && ${condition}` : condition; - } - - addPossibleIfCondition(): string { - const condition = this.getFullCondition(); - return condition ? `${condition} && ` : ''; - } - - addNestedIf(condition: string): void { - const ifScope = new IfScope({ condition, type: IfType.If }, this); - this.child = ifScope; - } - - addElseIf(condition: string): void { - this.current = { - condition, - parent: this.current as IfCondition | ElseIfCondition, - type: IfType.ElseIf - }; - } - - addElse(): void { - this.current = { parent: this.current as IfCondition | ElseIfCondition, type: IfType.Else }; - } - - getChild(): IfScope { - return this.child || this; - } - - getParent(): IfScope { - return this.parent || this; - } -} +import { IfScope } from './if-scope'; /** * {# if ...}...{/if} ---> {() => {if(...){<>...}}} @@ -108,7 +15,7 @@ export function handleIf(htmlx: string, str: MagicString, ifBlock: Node, ifScope str.overwrite(elseIfStart, ifBlock.expression.start, ' : (', { contentOnly: true }); str.overwrite(ifBlock.expression.end, elseIfConditionEnd, ') ? <>'); - ifScope.addElseIf(str.original.substring(ifBlock.expression.start, ifBlock.expression.end)); + ifScope.addElseIf(ifBlock.expression, str); if (!ifBlock.else) { str.appendLeft(endIf, ' : <>'); @@ -121,7 +28,7 @@ export function handleIf(htmlx: string, str: MagicString, ifBlock: Node, ifScope const end = htmlx.indexOf('}', ifBlock.expression.end); str.overwrite(ifBlock.expression.end, end + 1, ') ? <>', { contentOnly: true }); - ifScope.addNestedIf(str.original.substring(ifBlock.expression.start, ifBlock.expression.end)); + ifScope.addNestedIf(ifBlock.expression, str); if (ifBlock.else) { // {/if} -> } diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts new file mode 100644 index 000000000..4e1aebeb0 --- /dev/null +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts @@ -0,0 +1,279 @@ +import MagicString from 'magic-string'; +import { Node } from 'estree-walker'; +import { getIdentifiersInIfExpression } from '../utils/node-utils'; +import TemplateScope from '../../svelte2tsx/nodes/TemplateScope'; + +enum IfType { + If, + ElseIf, + Else +} + +interface ConditionInfo { + identifiers: Map; + condition: string; +} + +interface IfCondition { + type: IfType.If; + condition: ConditionInfo; +} + +interface ElseIfCondition { + type: IfType.ElseIf; + condition: ConditionInfo; + parent: IfCondition | ElseIfCondition; +} + +interface ElseCondition { + type: IfType.Else; + parent: IfCondition | ElseIfCondition; +} +type Condition = IfCondition | ElseIfCondition | ElseCondition; + +const REPLACEMENT_PREFIX = '\u03A9'; + +function getFullCondition( + condition: Condition, + replacedNames: string[], + replacementPrefix: string +): string { + switch (condition.type) { + case IfType.If: + return _getFullCondition(condition, false, replacedNames, replacementPrefix); + case IfType.ElseIf: + return _getFullCondition(condition, false, replacedNames, replacementPrefix); + case IfType.Else: + return _getFullCondition(condition, false, replacedNames, replacementPrefix); + } +} + +function _getFullCondition( + condition: Condition, + negate: boolean, + replacedNames: string[], + replacementPrefix: string +): string { + switch (condition.type) { + case IfType.If: + return negate + ? `!(${getConditionString(condition.condition, replacedNames, replacementPrefix)})` + : `(${getConditionString(condition.condition, replacedNames, replacementPrefix)})`; + case IfType.ElseIf: + return `${_getFullCondition( + condition.parent, + true, + replacedNames, + replacementPrefix + )} && ${negate ? '!' : ''}(${getConditionString( + condition.condition, + replacedNames, + replacementPrefix + )})`; + case IfType.Else: + return `${_getFullCondition(condition.parent, true, replacedNames, replacementPrefix)}`; + } +} + +function getConditionString( + condition: ConditionInfo, + replacedNames: string[], + replacementPrefix: string +): string { + const replacements: { name: string; start: number; end: number }[] = []; + for (const name of replacedNames) { + const occurences = condition.identifiers.get(name); + if (occurences) { + for (const occurence of occurences) { + replacements.push({ ...occurence, name }); + } + } + } + + if (!replacements.length) { + return condition.condition; + } + + replacements.sort((r1, r2) => r1.start - r2.start); + return ( + condition.condition.substring(0, replacements[0].start) + + replacements + .map( + (replacement, idx) => + replacementPrefix + + replacement.name + + condition.condition.substring(replacement.end, replacements[idx + 1]?.start) + ) + .join('') + ); +} + +function collectReferencedIdentifiers(condition: Condition | undefined): Set { + const identifiers = new Set(); + let current = condition; + while (current) { + if (current.type === IfType.ElseIf || current.type === IfType.If) { + for (const identifier of current.condition.identifiers.keys()) { + identifiers.add(identifier); + } + } + current = + current.type === IfType.ElseIf || current.type === IfType.Else + ? current.parent + : undefined; + } + return identifiers; +} + +// TODO: +// - Use TemplateScope in IfScope to detect shadowed variables +// - In case of Shadowed variables, do a walk on the expression. Search for "Identifier" | "MemberExpression"[.object] +// - prepend some unichar to all found identifiers and add const x = y; before it + +export class IfScope { + private child?: IfScope; + private ownScope = this.scope.value; + private replacementPrefix = REPLACEMENT_PREFIX.repeat(this.computeDepth()); + + constructor( + private scope: { value: TemplateScope }, + private current?: Condition, + private parent?: IfScope + ) {} + + getFullCondition(): string { + if (!this.current) { + return ''; + } + + const parentCondition = this.parent?.getFullCondition(); + const condition = `(${getFullCondition( + this.current, + this.getReplacedNames(), + this.replacementPrefix + )})`; + return parentCondition ? `(${parentCondition}) && ${condition}` : condition; + } + + addPossibleIfCondition(): string { + const condition = this.getFullCondition(); + return condition ? `${condition} && ` : ''; + } + + addNestedIf(expression: Node, str: MagicString): void { + const condition = this.getConditionString(str, expression); + const ifScope = new IfScope(this.scope, { condition, type: IfType.If }, this); + this.child = ifScope; + } + + addElseIf(expression: Node, str: MagicString): void { + const condition = this.getConditionString(str, expression); + this.current = { + condition, + parent: this.current as IfCondition | ElseIfCondition, + type: IfType.ElseIf + }; + } + + addElse(): void { + this.current = { parent: this.current as IfCondition | ElseIfCondition, type: IfType.Else }; + } + + getChild(): IfScope { + return this.child || this; + } + + getParent(): IfScope { + return this.parent || this; + } + + collectReferencedIdentifiers(): Set { + const current = collectReferencedIdentifiers(this.current); + const parent = this.parent?.collectReferencedIdentifiers(); + if (parent) { + for (const identifier of parent) { + current.add(identifier); + } + } + return current; + } + + addConstsPrefixIfNecessary(): string { + const replacements = this.getNamesToRedeclare() + .map((identifier) => `${this.replacementPrefix + identifier}=${identifier}`) + .join(','); + return replacements ? `() => {const ${replacements};` : ''; + } + + addConstsSuffixIfNecessary(): string { + return this.getNamesToRedeclare().length ? '}' : ''; + } + + referencesIdentifier(name: string): boolean { + const current = collectReferencedIdentifiers(this.current); + if (current.has(name)) { + return true; + } + if (!this.parent || this.ownScope.inits.has(name)) { + return false; + } + return this.parent.referencesIdentifier(name); + } + + private getConditionString(str: MagicString, expression: Node) { + const identifiers = getIdentifiersInIfExpression(expression); + const condition = str.original.substring(expression.start, expression.end); + return { identifiers, condition }; + } + + private getNamesToRedeclare() { + return [...this.scope.value.inits.keys()].filter((init) => { + let parent = this.scope.value.parent; + while (parent && parent !== this.ownScope) { + if (parent.inits.has(init)) { + return false; + } + parent = parent.parent; + } + return this.referencesIdentifier(init); + }); + } + + private getReplacedNames() { + const referencedIdentifiers = this.collectReferencedIdentifiers(); + return [...referencedIdentifiers].filter((identifier) => + this.someChildScopeHasRedeclaredVariable(identifier) + ); + } + + private someChildScopeHasRedeclaredVariable(name: string) { + let scope = this.scope.value; + while (scope && scope !== this.ownScope) { + if (scope.inits.has(name)) { + return true; + } + scope = scope.parent; + } + return false; + } + + private computeDepth() { + let idx = 1; + let parent = this.ownScope.parent; + while (parent) { + idx++; + parent = parent.parent; + } + return idx; + } +} + +// TODO: wenn ein neuer Scope aufgeht, muss der IfScope das bei der Analyse und dem Replacement berücksichtigen und das sozusagen "resetten" +// Scope, der info enthält, was gerade zu überschreiben ist. scope wird bei jedem if und elseif und await und each und let neu gemacht +// SCope enthält die aktuell geshadowed-en Variablen. +// depth für ifScope damit er weiß wie viele Ω vorne dran +// templatescope hat eine depth +// ifscope kriegt die depth gesagt +// wenn nach re-if gefragt wird, guckt ifscope den templatescope von current(=max) bis sichselbst+1. wenn da redeclare von variable wird die ersetzt +// wenn neues await/each/slot, guckt der ifscope "darüber", ob bei sich selbst oder vater-ifs (bis zu templatescope wo redeclare) schon eine solche variable in der ifcondition war und wenn ja const x=y prepend. +// jeder if-scope braucht seinen direkten templatescope-vater diff --git a/packages/svelte2tsx/src/htmlxtojsx/utils/node-utils.ts b/packages/svelte2tsx/src/htmlxtojsx/utils/node-utils.ts index 4a1543f40..05c0c98dc 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/utils/node-utils.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/utils/node-utils.ts @@ -1,4 +1,4 @@ -import { Node } from 'estree-walker'; +import { Node, walk } from 'estree-walker'; export function getTypeForComponent(node: Node): string { if (node.name === 'svelte:component' || node.name === 'svelte:self') { @@ -32,3 +32,34 @@ export function isShortHandAttribute(attr: Node): boolean { export function isQuote(str: string): boolean { return str === '"' || str === "'"; } + +export function getIdentifiersInIfExpression( + expression: Node +): Map { + const offset = expression.start; + const identifiers = new Map(); + walk(expression, { + enter: (node, parent) => { + switch (node.type) { + case 'Identifier': + // parent.property === node => node is "prop" in "obj.prop" + // parent.callee === node => node is "fun" in "fun(..)" + if (parent?.property !== node && parent?.callee !== node) { + add(node); + } + break; + } + } + }); + + function add(node: Node) { + let entry = identifiers.get(node.name); + if (!entry) { + entry = []; + } + entry.push({ start: node.start - offset, end: node.end - offset }); + identifiers.set(node.name, entry); + } + + return identifiers; +} diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/input.svelte b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/input.svelte new file mode 100644 index 000000000..c6d34f4bc --- /dev/null +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/input.svelte @@ -0,0 +1,35 @@ +{#if hello} + {#await x then hello} + {hello} + {#await x then hello} + {#if hello} + {hello} + {/if} + {/await} + {/await} + {#if hi && bye} + {#await x then bye} + {bye} + {:catch hello} + {#if hello} + {hello} + {/if} + {/await} + {:else if cool} + {#await cool} + loading + {:then cool} + {#if cool} + {cool} + {/if} + {:catch cool} + z + {/await} + {:else} + {#await x then hello} + {#if hello} + {hello} + {/if} + {/await} + {/if} +{/if} diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/expected.jsx b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/expected.jsx index 8955e9127..2ce1c2950 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/expected.jsx +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/expected.jsx @@ -3,11 +3,11 @@ {y} })}} {(hi && bye) ? <> - {() => {let _$$p = (x); __sveltets_awaitThen(_$$p, (y) => {(((hello))) && ((hi && bye)) && <> - {y} - }, () => {(((hello))) && ((hi && bye)) && <> - z - })}} + {() => {let _$$p = (x); __sveltets_awaitThen(_$$p, (y) => {(((hello))) && ((hi && bye)) && <> + {y} + }, () => {(((hello))) && ((hi && bye)) && <> + z + })}} : (cool) ? <> {() => {let _$$p = (x); (((hello))) && (!(hi && bye) && (cool)) && <> loading diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/input.svelte b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/input.svelte index 1075a1446..f5e6f03f4 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/input.svelte +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/input.svelte @@ -3,11 +3,11 @@ {y} {/await} {#if hi && bye} - {#await x then y} - {y} - {:catch} - z - {/await} + {#await x then y} + {y} + {:catch} + z + {/await} {:else if cool} {#await x} loading diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/expected.jsx b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/expected.jsx new file mode 100644 index 000000000..7c2bb5115 --- /dev/null +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/expected.jsx @@ -0,0 +1,20 @@ +<>{(hello) ? <> + {() => {const Ωhello=hello;__sveltets_each(items, (hello,i) => (hello.id) && ((Ωhello)) && <> +
{hello}{i}
+ )}} + {(hi && bye) ? <> + {() => {const Ωbye=bye;__sveltets_each(items, (bye) => (((hello))) && ((hi && Ωbye)) && <> +
{bye}
+ )}} +

hi

+ + : (cool) ? <> + {() => {const Ωcool=cool;__sveltets_each(items, (item,cool) => (((hello))) && (!(hi && bye) && (Ωcool)) && <> +
{item}{cool}
+ )}} + : <> + {() => {const Ωhello=hello;__sveltets_each(items, (hello) => (((Ωhello))) && (!(hi && bye) && !(cool)) && <> +
{hello}
+ )}} + } + : <>} \ No newline at end of file diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/input.svelte b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/input.svelte new file mode 100644 index 000000000..ca2ebbae1 --- /dev/null +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/input.svelte @@ -0,0 +1,20 @@ +{#if hello} + {#each items as hello,i (hello.id)} +
{hello}{i}
+ {/each} + {#if hi && bye} + {#each items as bye} +
{bye}
+ {:else} +

hi

+ {/each} + {:else if cool} + {#each items as item,cool} +
{item}{cool}
+ {/each} + {:else} + {#each items as hello} +
{hello}
+ {/each} + {/if} +{/if} diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed.solo/expected.jsx b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed.solo/expected.jsx new file mode 100644 index 000000000..2d0256e94 --- /dev/null +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed.solo/expected.jsx @@ -0,0 +1,34 @@ +<>{(hello && hello1) ? <> + {() => {const Ωhello=hello;() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['default'];((Ωhello && hello1)) && <> + {hello} + {() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['default'];((Ωhello && hello1)) && <> + {(hello) ? <> + {hello} + : <>} + }} + {(hello) ? <> + {() => {const ΩΩhello=hello;() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['default'];(((Ωhello && hello1))) && ((ΩΩhello)) && <> + {(hello) ? <> + {hello} + : <>} + }}} + : <>} + }}} + {(hi && bye) ? <> + {() => {const Ωbye=bye;() => { let {foo:bye} = __sveltets_instanceOf(Comp).$$slot_def['default'];(((hello && hello1))) && ((hi && Ωbye)) && <> + {bye} + }}} + : (cool) ? <> + + {() => {const Ωcool=cool,Ωhello=hello;() => { let {cool, hello} = __sveltets_instanceOf(Comp).$$slot_def['named'];(((Ωhello && hello1))) && (!(hi && bye) && (Ωcool)) && <>
+ {hello} +
}}} +
+ : <> + + {() => {const Ωhello=hello;() => { let {foo:hello, hello1:other} = __sveltets_instanceOf(Comp).$$slot_def['named'];(((Ωhello && hello1))) && (!(hi && bye) && !(cool)) && <>
+ {hello} +
}}} +
+ } + : <>} \ No newline at end of file diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed.solo/input.svelte b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed.solo/input.svelte new file mode 100644 index 000000000..abbb1956c --- /dev/null +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed.solo/input.svelte @@ -0,0 +1,34 @@ +{#if hello && hello1} + + {hello} + + {#if hello} + {hello} + {/if} + + {#if hello} + + {#if hello} + {hello} + {/if} + + {/if} + + {#if hi && bye} + + {bye} + + {:else if cool} + +
+ {hello} +
+
+ {:else} + +
+ {hello} +
+
+ {/if} +{/if} From c171810b7bd155cf082496539214a3d0dd671f69 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Tue, 2 Mar 2021 17:27:49 +0100 Subject: [PATCH 03/17] fix svelte:fragment if-scoping --- packages/svelte2tsx/src/htmlxtojsx/index.ts | 21 ++++---- .../expected.jsx | 35 +++++++++++++ .../expected.jsx | 34 ------------- .../if-nested-slot-let-shadowed/expected.jsx | 49 +++++++++++++++++++ .../input.svelte | 15 ++++++ 5 files changed, 111 insertions(+), 43 deletions(-) create mode 100644 packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/expected.jsx delete mode 100644 packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed.solo/expected.jsx create mode 100644 packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed/expected.jsx rename packages/svelte2tsx/test/htmlx2jsx/samples/{if-nested-slot-let-shadowed.solo => if-nested-slot-let-shadowed}/input.svelte (65%) diff --git a/packages/svelte2tsx/src/htmlxtojsx/index.ts b/packages/svelte2tsx/src/htmlxtojsx/index.ts index 08ac9f2f5..e692c825e 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/index.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/index.ts @@ -152,15 +152,18 @@ export function convertHtmlxToJsx( break; case 'SlotTemplate': handleSvelteTag(htmlx, str, node); - handleSlot( - htmlx, - str, - node, - parent, - getSlotName(node) || 'default', - ifScope, - templateScope.value - ); + if (usesLet(node)) { + templateScope.value = templateScope.value.child(); + handleSlot( + htmlx, + str, + node, + parent, + getSlotName(node) || 'default', + ifScope, + templateScope.value + ); + } break; case 'Text': handleText(str, node); diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/expected.jsx b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/expected.jsx new file mode 100644 index 000000000..361a5c3f9 --- /dev/null +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/expected.jsx @@ -0,0 +1,35 @@ +<>{(hello) ? <> + {() => {let _$$p = (x); __sveltets_awaitThen(_$$p, (hello) => {((Ωhello)) && <> + {hello} + {() => {let _$$p = (x); __sveltets_awaitThen(_$$p, (hello) => {((Ωhello)) && <> + {(hello) ? <> + {hello} + : <>} + })}} + })}} + {(hi && bye) ? <> + {() => {let _$$p = (x); __sveltets_awaitThen(_$$p, (bye) => {(((Ωhello))) && ((hi && Ωbye)) && <> + {bye} + }, (hello) => {(((Ωhello))) && ((hi && Ωbye)) && <> + {(hello) ? <> + {hello} + : <>} + })}} + : (cool) ? <> + {() => {let _$$p = (cool); ((((hello))) && (!(hi && bye) && (Ωcool))) && ((hello)) && <> + loading + ; __sveltets_awaitThen(_$$p, (cool) => {((((hello))) && (!(hi && bye) && (Ωcool))) && ((hello)) && <> + {(cool) ? <> + {cool} + : <>} + }, (cool) => {((((hello))) && (!(hi && bye) && (Ωcool))) && ((hello)) && <> + z + })}} + : <> + {() => {let _$$p = (x); __sveltets_awaitThen(_$$p, (hello) => {((((Ωhello))) && (!(hi && bye) && (cool))) && (!(ΩΩhello)) && <> + {(hello) ? <> + {hello} + : <>} + })}} + } + : <>} \ No newline at end of file diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed.solo/expected.jsx b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed.solo/expected.jsx deleted file mode 100644 index 2d0256e94..000000000 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed.solo/expected.jsx +++ /dev/null @@ -1,34 +0,0 @@ -<>{(hello && hello1) ? <> - {() => {const Ωhello=hello;() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['default'];((Ωhello && hello1)) && <> - {hello} - {() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['default'];((Ωhello && hello1)) && <> - {(hello) ? <> - {hello} - : <>} - }} - {(hello) ? <> - {() => {const ΩΩhello=hello;() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['default'];(((Ωhello && hello1))) && ((ΩΩhello)) && <> - {(hello) ? <> - {hello} - : <>} - }}} - : <>} - }}} - {(hi && bye) ? <> - {() => {const Ωbye=bye;() => { let {foo:bye} = __sveltets_instanceOf(Comp).$$slot_def['default'];(((hello && hello1))) && ((hi && Ωbye)) && <> - {bye} - }}} - : (cool) ? <> - - {() => {const Ωcool=cool,Ωhello=hello;() => { let {cool, hello} = __sveltets_instanceOf(Comp).$$slot_def['named'];(((Ωhello && hello1))) && (!(hi && bye) && (Ωcool)) && <>
- {hello} -
}}} -
- : <> - - {() => {const Ωhello=hello;() => { let {foo:hello, hello1:other} = __sveltets_instanceOf(Comp).$$slot_def['named'];(((Ωhello && hello1))) && (!(hi && bye) && !(cool)) && <>
- {hello} -
}}} -
- } - : <>} \ No newline at end of file diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed/expected.jsx b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed/expected.jsx new file mode 100644 index 000000000..00552c763 --- /dev/null +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed/expected.jsx @@ -0,0 +1,49 @@ +<>{(hello && hello1) ? <> + {() => {const Ωhello=hello;() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['default'];((Ωhello && hello1)) && <> + {hello} + {() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['default'];((Ωhello && hello1)) && <> + {(hello) ? <> + {hello} + : <>} + }} + {() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['named1'];((Ωhello && hello1)) && <> + {(hello) ? <> + {hello} + : <>} + }} + {() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['named2'];((Ωhello && hello1)) && <>

+ {(hello) ? <> + {hello} + : <>} +

}} + {() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['named3'];((Ωhello && hello1)) && <> + {(hello) ? <> + {hello} + : <>} + }} + {(hello) ? <> + {() => {const ΩΩΩhello=hello;() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['default'];(((Ωhello && hello1))) && ((ΩΩΩhello)) && <> + {(hello) ? <> + {hello} + : <>} + }}} + : <>} + }}}
+ {(hi && bye) ? <> + {() => {const ΩΩbye=bye;() => { let {foo:bye} = __sveltets_instanceOf(Comp).$$slot_def['default'];(((Ωhello && hello1))) && ((hi && ΩΩbye)) && <> + {bye} + }}} + : (cool) ? <> + + {() => {const ΩΩcool=cool;() => { let {cool, hello} = __sveltets_instanceOf(Comp).$$slot_def['named'];(((Ωhello && hello1))) && (!(hi && bye) && (ΩΩcool)) && <>
+ {hello} +
}}} +
+ : <> + + {() => { let {foo:hello, hello1:other} = __sveltets_instanceOf(Comp).$$slot_def['named'];(((Ωhello && hello1))) && (!(hi && bye) && !(cool)) && <>
+ {hello} +
}} +
+ } + : <>} \ No newline at end of file diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed.solo/input.svelte b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed/input.svelte similarity index 65% rename from packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed.solo/input.svelte rename to packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed/input.svelte index abbb1956c..3a4cc626e 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed.solo/input.svelte +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed/input.svelte @@ -6,6 +6,21 @@ {hello} {/if} + + {#if hello} + {hello} + {/if} + +

+ {#if hello} + {hello} + {/if} +

+ + {#if hello} + {hello} + {/if} + {#if hello} {#if hello} From 5854a125f801c6425d0cf1df5f9f956ab93b9174 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Tue, 2 Mar 2021 17:37:47 +0100 Subject: [PATCH 04/17] each --- .../svelte2tsx/src/htmlxtojsx/nodes/each.ts | 14 +++++----- .../src/htmlxtojsx/nodes/if-scope.ts | 4 +-- .../svelte2tsx/src/htmlxtojsx/nodes/slot.ts | 7 +++-- .../expected.jsx | 27 ++++++++++++------- .../input.svelte | 11 +++++++- 5 files changed, 42 insertions(+), 21 deletions(-) diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/each.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/each.ts index 04581a01d..174beca87 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/each.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/each.ts @@ -13,11 +13,9 @@ export function handleEach( ): void { // {#each items as item,i (key)} -> // {__sveltets_each(items, (item,i) => (key) && (possible if expression &&) <> - str.overwrite( - eachBlock.start, - eachBlock.expression.start, - `{${ifScope.addConstsPrefixIfNecessary()}() => {__sveltets_each(` - ); + const constRedeclares = ifScope.getConstsToRedeclare(); + const prefix = constRedeclares ? `{() => {${constRedeclares}() => ` : ''; + str.overwrite(eachBlock.start, eachBlock.expression.start, `${prefix}{__sveltets_each(`); str.overwrite(eachBlock.expression.end, eachBlock.context.start, ', ('); // {#each true, items as item} @@ -39,14 +37,16 @@ export function handleEach( const endEachStart = htmlx.indexOf('}', contextEnd); str.overwrite(endEachStart, endEachStart + 1, ` ${ifScope.addPossibleIfCondition()}<>`); } + const endEach = htmlx.lastIndexOf('{', eachBlock.end - 1); + const suffix = constRedeclares ? ')}}}' : ')}'; // {/each} -> )} or {:else} -> )} if (eachBlock.else) { const elseEnd = htmlx.lastIndexOf('}', eachBlock.else.start); const elseStart = htmlx.lastIndexOf('{', elseEnd); - str.overwrite(elseStart, elseEnd + 1, `)}}${ifScope.addConstsSuffixIfNecessary()}`); + str.overwrite(elseStart, elseEnd + 1, suffix); str.remove(endEach, eachBlock.end); } else { - str.overwrite(endEach, eachBlock.end, `)}}${ifScope.addConstsSuffixIfNecessary()}`); + str.overwrite(endEach, eachBlock.end, suffix); } } diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts index 4e1aebeb0..f553f908e 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts @@ -198,11 +198,11 @@ export class IfScope { return current; } - addConstsPrefixIfNecessary(): string { + getConstsToRedeclare(): string { const replacements = this.getNamesToRedeclare() .map((identifier) => `${this.replacementPrefix + identifier}=${identifier}`) .join(','); - return replacements ? `() => {const ${replacements};` : ''; + return replacements ? `const ${replacements};` : ''; } addConstsSuffixIfNecessary(): string { diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/slot.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/slot.ts index 278956476..bca82650f 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/slot.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/slot.ts @@ -62,7 +62,10 @@ export function handleSlot( if (!hasMoved) { return; } - str.appendLeft(slotDefInsertionPoint, `{${ifScope.addConstsPrefixIfNecessary()}() => { let {`); + + const constRedeclares = ifScope.getConstsToRedeclare(); + const prefix = constRedeclares ? `() => {${constRedeclares}` : ''; + str.appendLeft(slotDefInsertionPoint, `{${prefix}() => { let {`); str.appendRight( slotDefInsertionPoint, `} = ${getSingleSlotDef(component, slotName)}` + `;${ifScope.addPossibleIfCondition()}<>` @@ -71,7 +74,7 @@ export function handleSlot( const closeSlotDefInsertionPoint = slotElIsComponent ? htmlx.lastIndexOf('<', slotEl.end - 1) : slotEl.end; - str.appendLeft(closeSlotDefInsertionPoint, `}}${ifScope.addConstsSuffixIfNecessary()}`); + str.appendLeft(closeSlotDefInsertionPoint, `}}${constRedeclares ? '}' : ''}`); } export function usesLet(node: Node) { diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/expected.jsx b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/expected.jsx index 7c2bb5115..d538ba8a6 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/expected.jsx +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/expected.jsx @@ -1,20 +1,29 @@ <>{(hello) ? <> - {() => {const Ωhello=hello;__sveltets_each(items, (hello,i) => (hello.id) && ((Ωhello)) && <> + {() => {const Ωhello=hello;() => {__sveltets_each(items, (hello,i) => (hello.id) && ((Ωhello)) && <>
{hello}{i}
- )}} + {(hello) ? <> + {() => {const ΩΩhello=hello;() => {__sveltets_each(items, (hello) => (((Ωhello))) && ((ΩΩhello)) && <> + {(hello) ? <> + {hello} + : <>} + )}}} + : <>} + )}}} {(hi && bye) ? <> - {() => {const Ωbye=bye;__sveltets_each(items, (bye) => (((hello))) && ((hi && Ωbye)) && <> + {() => {const Ωbye=bye;() => {__sveltets_each(items, (bye) => (((hello))) && ((hi && Ωbye)) && <>
{bye}
- )}} -

hi

+ )}}} + {(bye) ? <> + {bye} + : <>} : (cool) ? <> - {() => {const Ωcool=cool;__sveltets_each(items, (item,cool) => (((hello))) && (!(hi && bye) && (Ωcool)) && <> + {() => {const ΩΩcool=cool;() => {__sveltets_each(items, (item,cool) => ((((hello))) && (!(hi && bye) && (Ωcool))) && ((bye)) && <>
{item}{cool}
- )}} + )}}} : <> - {() => {const Ωhello=hello;__sveltets_each(items, (hello) => (((Ωhello))) && (!(hi && bye) && !(cool)) && <> + {() => {const ΩΩhello=hello;() => {__sveltets_each(items, (hello) => ((((Ωhello))) && (!(hi && bye) && (cool))) && (!(bye)) && <>
{hello}
- )}} + )}}} } : <>} \ No newline at end of file diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/input.svelte b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/input.svelte index ca2ebbae1..bf121797e 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/input.svelte +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/input.svelte @@ -1,12 +1,21 @@ {#if hello} {#each items as hello,i (hello.id)}
{hello}{i}
+ {#if hello} + {#each items as hello} + {#if hello} + {hello} + {/if} + {/each} + {/if} {/each} {#if hi && bye} {#each items as bye}
{bye}
{:else} -

hi

+ {#if bye} + {bye} + {/if} {/each} {:else if cool} {#each items as item,cool} From 19c55a40bec02647650d6ec8e08e5f21b54c66e3 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 3 Mar 2021 11:02:41 +0100 Subject: [PATCH 05/17] fix scoping, get tests passing. TODO: cleanup --- packages/svelte2tsx/src/htmlxtojsx/index.ts | 12 ++++- .../svelte2tsx/src/htmlxtojsx/nodes/await.ts | 7 ++- .../expected.jsx | 46 ++++++++++++++----- .../input.svelte | 34 +++++++++++--- .../expected.jsx | 28 +++++++++-- .../input.svelte | 22 +++++++++ .../if-nested-slot-let-shadowed/expected.jsx | 40 +++++++++++++--- .../if-nested-slot-let-shadowed/input.svelte | 28 +++++++++++ 8 files changed, 188 insertions(+), 29 deletions(-) diff --git a/packages/svelte2tsx/src/htmlxtojsx/index.ts b/packages/svelte2tsx/src/htmlxtojsx/index.ts index e692c825e..bccec2796 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/index.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/index.ts @@ -80,13 +80,18 @@ export function convertHtmlxToJsx( switch (node.type) { case 'IfBlock': handleIf(htmlx, str, node, ifScope); - ifScope = ifScope.getChild(); + if (!node.elseif) { + ifScope = ifScope.getChild(); + } break; case 'EachBlock': handleScopeEach(node); handleEach(htmlx, str, node, ifScope); break; case 'ElseBlock': + if (parent.type === 'EachBlock') { + templateScope.value = templateScope.value.parent; + } handleElse(htmlx, str, node, parent, ifScope); break; case 'AwaitBlock': @@ -185,11 +190,16 @@ export function convertHtmlxToJsx( ifScope = ifScope.getParent(); break; case 'EachBlock': + if (!node.else) { + templateScope.value = templateScope.value.parent; + } + break; case 'AwaitBlock': templateScope.value = templateScope.value.parent; break; case 'InlineComponent': case 'Element': + case 'SlotTemplate': if (usesLet(node)) { templateScope.value = templateScope.value.parent; } diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/await.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/await.ts index 073ae78ba..63cae1003 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/await.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/await.ts @@ -13,7 +13,12 @@ export function handleAwait( ): void { // {#await somePromise then value} -> // {() => {let _$$p = (somePromise); - str.overwrite(awaitBlock.start, awaitBlock.expression.start, '{() => {let _$$p = ('); + const constRedeclares = ifScope.getConstsToRedeclare(); + str.overwrite( + awaitBlock.start, + awaitBlock.expression.start, + `{() => {${constRedeclares}let _$$p = (` + ); // then value } | {:then value} | {await ..} .. {/await} -> // __sveltets_awaitThen(_$$p, (value) => {(possibleIfCondition && )<> diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/expected.jsx b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/expected.jsx index 361a5c3f9..cb74c9fa8 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/expected.jsx +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/expected.jsx @@ -1,14 +1,16 @@ <>{(hello) ? <> - {() => {let _$$p = (x); __sveltets_awaitThen(_$$p, (hello) => {((Ωhello)) && <> + {() => {const Ωhello=hello;let _$$p = (hello); __sveltets_awaitThen(_$$p, (hello) => {((Ωhello)) && <> {hello} - {() => {let _$$p = (x); __sveltets_awaitThen(_$$p, (hello) => {((Ωhello)) && <> - {(hello) ? <> - {hello} - : <>} - })}} + {(hello) ? <> + {() => {const ΩΩhello=hello;let _$$p = (x); __sveltets_awaitThen(_$$p, (hello) => {(((Ωhello))) && ((ΩΩhello)) && <> + {(hello) ? <> + {hello} + : <>} + })}} + : <>} })}} {(hi && bye) ? <> - {() => {let _$$p = (x); __sveltets_awaitThen(_$$p, (bye) => {(((Ωhello))) && ((hi && Ωbye)) && <> + {() => {const Ωbye=bye,Ωhello=hello;let _$$p = (x); __sveltets_awaitThen(_$$p, (bye) => {(((Ωhello))) && ((hi && Ωbye)) && <> {bye} }, (hello) => {(((Ωhello))) && ((hi && Ωbye)) && <> {(hello) ? <> @@ -16,20 +18,40 @@ : <>} })}} : (cool) ? <> - {() => {let _$$p = (cool); ((((hello))) && (!(hi && bye) && (Ωcool))) && ((hello)) && <> + {() => {const Ωcool=cool;let _$$p = (cool); (((hello))) && (!(hi && bye) && (Ωcool)) && <> loading - ; __sveltets_awaitThen(_$$p, (cool) => {((((hello))) && (!(hi && bye) && (Ωcool))) && ((hello)) && <> + ; __sveltets_awaitThen(_$$p, (cool) => {(((hello))) && (!(hi && bye) && (Ωcool)) && <> {(cool) ? <> {cool} : <>} - }, (cool) => {((((hello))) && (!(hi && bye) && (Ωcool))) && ((hello)) && <> + }, (cool) => {(((hello))) && (!(hi && bye) && (Ωcool)) && <> z })}} : <> - {() => {let _$$p = (x); __sveltets_awaitThen(_$$p, (hello) => {((((Ωhello))) && (!(hi && bye) && (cool))) && (!(ΩΩhello)) && <> + {() => {const Ωhello=hello;let _$$p = (x); __sveltets_awaitThen(_$$p, (hello) => {(((Ωhello))) && (!(hi && bye) && !(cool)) && <> {(hello) ? <> {hello} : <>} })}} } - : <>} \ No newline at end of file + : <>} + +{() => {let _$$p = (cool); <> + {(cool) ? <> + {cool} + : (hello) ? <> + {hello} + : <> } +; __sveltets_awaitThen(_$$p, (cool) => {<> + {(cool) ? <> + {cool} + : (hello) ? <> + {hello} + : <> } +}, (cool) => {<> + {(cool) ? <> + {cool} + : (hello) ? <> + {hello} + : <> } +})}} \ No newline at end of file diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/input.svelte b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/input.svelte index c6d34f4bc..116faa090 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/input.svelte +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/input.svelte @@ -1,11 +1,13 @@ {#if hello} - {#await x then hello} + {#await hello then hello} {hello} - {#await x then hello} - {#if hello} - {hello} - {/if} - {/await} + {#if hello} + {#await x then hello} + {#if hello} + {hello} + {/if} + {/await} + {/if} {/await} {#if hi && bye} {#await x then bye} @@ -33,3 +35,23 @@ {/await} {/if} {/if} + +{#await cool} + {#if cool} + {cool} + {:else if hello} + {hello} + {/if} +{:then cool} + {#if cool} + {cool} + {:else if hello} + {hello} + {/if} +{:catch cool} + {#if cool} + {cool} + {:else if hello} + {hello} + {/if} +{/await} \ No newline at end of file diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/expected.jsx b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/expected.jsx index d538ba8a6..e1760eb75 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/expected.jsx +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/expected.jsx @@ -9,6 +9,10 @@ )}}} : <>} )}}} + {(hello) ? <> + {hello} + : <>} + {(hi && bye) ? <> {() => {const Ωbye=bye;() => {__sveltets_each(items, (bye) => (((hello))) && ((hi && Ωbye)) && <>
{bye}
@@ -18,12 +22,30 @@ : <>} : (cool) ? <> - {() => {const ΩΩcool=cool;() => {__sveltets_each(items, (item,cool) => ((((hello))) && (!(hi && bye) && (Ωcool))) && ((bye)) && <> + {() => {const Ωcool=cool;() => {__sveltets_each(items, (item,cool) => (((hello))) && (!(hi && bye) && (Ωcool)) && <>
{item}{cool}
)}}} : <> - {() => {const ΩΩhello=hello;() => {__sveltets_each(items, (hello) => ((((Ωhello))) && (!(hi && bye) && (cool))) && (!(bye)) && <> + {() => {const Ωhello=hello;() => {__sveltets_each(items, (hello) => (((Ωhello))) && (!(hi && bye) && !(cool)) && <>
{hello}
)}}} } - : <>} \ No newline at end of file + : <>} + +{__sveltets_each(items, (hello,i) => <> + {(hello && i && bye) ? <> + {hello} {i} {bye} + : (hello && i && bye) ? <> + {hello} {i} {bye} + : <> + {hello} {i} {bye} + } +)} + {(hello && i && bye) ? <> + {hello} {i} {bye} + : (hello && i && bye) ? <> + {hello} {i} {bye} + : <> + {hello} {i} {bye} + } + \ No newline at end of file diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/input.svelte b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/input.svelte index bf121797e..5391a4393 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/input.svelte +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/input.svelte @@ -8,6 +8,10 @@ {/if} {/each} {/if} + {:else} + {#if hello} + {hello} + {/if} {/each} {#if hi && bye} {#each items as bye} @@ -27,3 +31,21 @@ {/each} {/if} {/if} + +{#each items as hello,i} + {#if hello && i && bye} + {hello} {i} {bye} + {:else if hello && i && bye} + {hello} {i} {bye} + {:else} + {hello} {i} {bye} + {/if} +{:else} + {#if hello && i && bye} + {hello} {i} {bye} + {:else if hello && i && bye} + {hello} {i} {bye} + {:else} + {hello} {i} {bye} + {/if} +{/each} \ No newline at end of file diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed/expected.jsx b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed/expected.jsx index 00552c763..b77eebc42 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed/expected.jsx +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed/expected.jsx @@ -22,7 +22,7 @@ : <>}
}} {(hello) ? <> - {() => {const ΩΩΩhello=hello;() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['default'];(((Ωhello && hello1))) && ((ΩΩΩhello)) && <> + {() => {const ΩΩhello=hello;() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['default'];(((Ωhello && hello1))) && ((ΩΩhello)) && <> {(hello) ? <> {hello} : <>} @@ -30,20 +30,48 @@ : <>} }}} {(hi && bye) ? <> - {() => {const ΩΩbye=bye;() => { let {foo:bye} = __sveltets_instanceOf(Comp).$$slot_def['default'];(((Ωhello && hello1))) && ((hi && ΩΩbye)) && <> + {() => {const Ωbye=bye;() => { let {foo:bye} = __sveltets_instanceOf(Comp).$$slot_def['default'];(((hello && hello1))) && ((hi && Ωbye)) && <> {bye} }}} : (cool) ? <> - {() => {const ΩΩcool=cool;() => { let {cool, hello} = __sveltets_instanceOf(Comp).$$slot_def['named'];(((Ωhello && hello1))) && (!(hi && bye) && (ΩΩcool)) && <>
+ {() => {const Ωcool=cool,Ωhello=hello;() => { let {cool, hello} = __sveltets_instanceOf(Comp).$$slot_def['named'];(((Ωhello && hello1))) && (!(hi && bye) && (Ωcool)) && <>
{hello}
}}} : <> - {() => { let {foo:hello, hello1:other} = __sveltets_instanceOf(Comp).$$slot_def['named'];(((Ωhello && hello1))) && (!(hi && bye) && !(cool)) && <>
+ {() => {const Ωhello=hello;() => { let {foo:hello, hello1:other} = __sveltets_instanceOf(Comp).$$slot_def['named'];(((Ωhello && hello1))) && (!(hi && bye) && !(cool)) && <>
{hello} -
}} +
}}}
} - : <>} \ No newline at end of file + : <>} + +{() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['default'];<> + {(hello && bye) ? <> + {hello} {bye} + : (hello && bye) ? <> + {hello} {bye} + : <> + {hello} {bye} + } + {() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['named1'];<> + {(hello && bye) ? <> + {hello} {bye} + : (hello && bye) ? <> + {hello} {bye} + : <> + {hello} {bye} + } + }} + {() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['named2'];<>

+ {(hello && bye) ? <> + {hello} {bye} + : (hello && bye) ? <> + {hello} {bye} + : <> + {hello} {bye} + } +

}} +}}
\ No newline at end of file diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed/input.svelte b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed/input.svelte index 3a4cc626e..6d3489916 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed/input.svelte +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed/input.svelte @@ -47,3 +47,31 @@ {/if} {/if} + + + {#if hello && bye} + {hello} {bye} + {:else if hello && bye} + {hello} {bye} + {:else} + {hello} {bye} + {/if} + + {#if hello && bye} + {hello} {bye} + {:else if hello && bye} + {hello} {bye} + {:else} + {hello} {bye} + {/if} + +

+ {#if hello && bye} + {hello} {bye} + {:else if hello && bye} + {hello} {bye} + {:else} + {hello} {bye} + {/if} +

+
\ No newline at end of file From ff389149d8a00ffaea735591df38286b40c6562b Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 3 Mar 2021 11:15:26 +0100 Subject: [PATCH 06/17] use own template scope --- packages/svelte2tsx/src/htmlxtojsx/index.ts | 16 ++++++------- .../src/htmlxtojsx/nodes/component.ts | 2 +- .../src/htmlxtojsx/nodes/element.ts | 2 +- .../src/htmlxtojsx/nodes/if-scope.ts | 2 +- .../svelte2tsx/src/htmlxtojsx/nodes/slot.ts | 10 ++------ .../src/htmlxtojsx/nodes/template-scope.ts | 23 +++++++++++++++++++ 6 files changed, 36 insertions(+), 19 deletions(-) create mode 100644 packages/svelte2tsx/src/htmlxtojsx/nodes/template-scope.ts diff --git a/packages/svelte2tsx/src/htmlxtojsx/index.ts b/packages/svelte2tsx/src/htmlxtojsx/index.ts index bccec2796..80104318b 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/index.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/index.ts @@ -22,7 +22,7 @@ import { handleRawHtml } from './nodes/raw-html'; import { handleSvelteTag } from './nodes/svelte-tag'; import { handleTransitionDirective } from './nodes/transition-directive'; import { handleText } from './nodes/text'; -import TemplateScope from '../svelte2tsx/nodes/TemplateScope'; +import TemplateScope from './nodes/template-scope'; import { getSlotName, isDestructuringPatterns, isIdentifier } from '../utils/svelteAst'; import { extract_identifiers } from 'periscopic'; import { SvelteIdentifier } from '../interfaces'; @@ -56,19 +56,19 @@ export function convertHtmlxToJsx( const handleScopeEach = (node: Node) => { templateScope.value = templateScope.value.child(); if (node.context) { - handleScope(node.context, node, templateScope.value); + handleScope(node.context, templateScope.value); } if (node.index) { - templateScope.value.add({ name: node.index, type: 'Identifier' }, node); + templateScope.value.add(node.index); } }; const handleScopeAwait = (node: Node) => { templateScope.value = templateScope.value.child(); if (node.value) { - handleScope(node.value, node.then, templateScope.value); + handleScope(node.value, templateScope.value); } if (node.error) { - handleScope(node.error, node.catch, templateScope.value); + handleScope(node.error, templateScope.value); } }; @@ -216,14 +216,14 @@ export function convertHtmlxToJsx( }); } -function handleScope(identifierDef: Node, owner: Node, templateScope: TemplateScope) { +function handleScope(identifierDef: Node, templateScope: TemplateScope) { if (isIdentifier(identifierDef)) { - templateScope.add(identifierDef, owner); + templateScope.add(identifierDef.name); } if (isDestructuringPatterns(identifierDef)) { // the node object is returned as-it with no mutation const identifiers = extract_identifiers(identifierDef) as SvelteIdentifier[]; - templateScope.addMany(identifiers, owner); + templateScope.addMany(identifiers.map((id) => id.name)); } } diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/component.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/component.ts index 3eeab9134..ed55e6c02 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/component.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/component.ts @@ -3,7 +3,7 @@ import { Node } from 'estree-walker'; import { getSlotName } from '../../utils/svelteAst'; import { handleSlot } from './slot'; import { IfScope } from './if-scope'; -import TemplateScope from '../../svelte2tsx/nodes/TemplateScope'; +import TemplateScope from '../nodes/template-scope'; /** * Handle `` and slot-specific transformations. diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/element.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/element.ts index 6d1fd14e7..6e6454274 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/element.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/element.ts @@ -3,7 +3,7 @@ import { Node } from 'estree-walker'; import { getSlotName } from '../../utils/svelteAst'; import { handleSlot } from './slot'; import { IfScope } from './if-scope'; -import TemplateScope from '../../svelte2tsx/nodes/TemplateScope'; +import TemplateScope from '../nodes/template-scope'; /** * Special treatment for self-closing / void tags to make them conform to JSX. diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts index f553f908e..ab6171103 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts @@ -1,7 +1,7 @@ import MagicString from 'magic-string'; import { Node } from 'estree-walker'; import { getIdentifiersInIfExpression } from '../utils/node-utils'; -import TemplateScope from '../../svelte2tsx/nodes/TemplateScope'; +import TemplateScope from '../nodes/template-scope'; enum IfType { If, diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/slot.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/slot.ts index bca82650f..df446c208 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/slot.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/slot.ts @@ -3,7 +3,7 @@ import { Node } from 'estree-walker'; import { beforeStart } from '../utils/node-utils'; import { getSingleSlotDef } from '../../svelte2tsx/nodes/slot'; import { IfScope } from './if-scope'; -import TemplateScope from '../../svelte2tsx/nodes/TemplateScope'; +import TemplateScope from '../nodes/template-scope'; export function handleSlot( htmlx: string, @@ -43,13 +43,7 @@ export function handleSlot( } else { str.remove(attr.start, attr.start + 'let:'.length); } - templateScope.add( - { - name: attr.expression?.name || attr.name, - type: 'Identifier' - }, - slotEl - ); + templateScope.add(attr.expression?.name || attr.name); hasMoved = true; if (attr.expression) { //overwrite the = as a : diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/template-scope.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/template-scope.ts new file mode 100644 index 000000000..e97fb92bc --- /dev/null +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/template-scope.ts @@ -0,0 +1,23 @@ +export default class TemplateScope { + inits = new Set(); + parent?: TemplateScope; + + constructor(parent?: TemplateScope) { + this.parent = parent; + } + + addMany(inits: string[]) { + inits.forEach((item) => this.add(item)); + return this; + } + + add(name: string) { + this.inits.add(name); + return this; + } + + child() { + const child = new TemplateScope(this); + return child; + } +} From ea1409483cb741d611bb592919d1b11be6a39b75 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 3 Mar 2021 11:32:17 +0100 Subject: [PATCH 07/17] template scope manager --- packages/svelte2tsx/src/htmlxtojsx/index.ts | 94 +++++++------------ .../src/htmlxtojsx/nodes/component.ts | 2 +- .../src/htmlxtojsx/nodes/element.ts | 2 +- .../src/htmlxtojsx/nodes/if-scope.ts | 2 +- .../svelte2tsx/src/htmlxtojsx/nodes/slot.ts | 6 +- .../src/htmlxtojsx/nodes/template-scope.ts | 71 +++++++++++++- .../src/htmlxtojsx/utils/node-utils.ts | 4 + 7 files changed, 112 insertions(+), 69 deletions(-) diff --git a/packages/svelte2tsx/src/htmlxtojsx/index.ts b/packages/svelte2tsx/src/htmlxtojsx/index.ts index 80104318b..94d0f095c 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/index.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/index.ts @@ -2,30 +2,29 @@ import { Node } from 'estree-walker'; import MagicString from 'magic-string'; import svelte from 'svelte/compiler'; import { parseHtmlx } from '../utils/htmlxparser'; +import { getSlotName } from '../utils/svelteAst'; import { handleActionDirective } from './nodes/action-directive'; import { handleAnimateDirective } from './nodes/animation-directive'; import { handleAttribute } from './nodes/attribute'; import { handleAwait } from './nodes/await'; -import { handleKey } from './nodes/key'; import { handleBinding } from './nodes/binding'; import { handleClassDirective } from './nodes/class-directive'; import { handleComment } from './nodes/comment'; import { handleComponent } from './nodes/component'; -import { handleSlot, usesLet } from './nodes/slot'; import { handleDebug } from './nodes/debug'; import { handleEach } from './nodes/each'; import { handleElement } from './nodes/element'; import { handleEventHandler } from './nodes/event-handler'; import { handleElse, handleIf } from './nodes/if-else'; import { IfScope } from './nodes/if-scope'; +import { handleKey } from './nodes/key'; import { handleRawHtml } from './nodes/raw-html'; +import { handleSlot } from './nodes/slot'; import { handleSvelteTag } from './nodes/svelte-tag'; -import { handleTransitionDirective } from './nodes/transition-directive'; +import { TemplateScopeManager } from './nodes/template-scope'; import { handleText } from './nodes/text'; -import TemplateScope from './nodes/template-scope'; -import { getSlotName, isDestructuringPatterns, isIdentifier } from '../utils/svelteAst'; -import { extract_identifiers } from 'periscopic'; -import { SvelteIdentifier } from '../interfaces'; +import { handleTransitionDirective } from './nodes/transition-directive'; +import { usesLet } from './utils/node-utils'; type Walker = (node: Node, parent: Node, prop: string, index: number) => void; @@ -52,27 +51,9 @@ export function convertHtmlxToJsx( str.prepend('<>'); str.append(''); - const templateScope = { value: new TemplateScope() }; - const handleScopeEach = (node: Node) => { - templateScope.value = templateScope.value.child(); - if (node.context) { - handleScope(node.context, templateScope.value); - } - if (node.index) { - templateScope.value.add(node.index); - } - }; - const handleScopeAwait = (node: Node) => { - templateScope.value = templateScope.value.child(); - if (node.value) { - handleScope(node.value, templateScope.value); - } - if (node.error) { - handleScope(node.error, templateScope.value); - } - }; + const templateScopeManager = new TemplateScopeManager(); - let ifScope = new IfScope(templateScope); + let ifScope = new IfScope(templateScopeManager); (svelte as any).walk(ast, { enter: (node: Node, parent: Node, prop: string, index: number) => { @@ -85,17 +66,15 @@ export function convertHtmlxToJsx( } break; case 'EachBlock': - handleScopeEach(node); + templateScopeManager.eachEnter(node); handleEach(htmlx, str, node, ifScope); break; case 'ElseBlock': - if (parent.type === 'EachBlock') { - templateScope.value = templateScope.value.parent; - } + templateScopeManager.elseEnter(parent); handleElse(htmlx, str, node, parent, ifScope); break; case 'AwaitBlock': - handleScopeAwait(node); + templateScopeManager.awaitEnter(node); handleAwait(htmlx, str, node, ifScope); break; case 'KeyBlock': @@ -108,16 +87,26 @@ export function convertHtmlxToJsx( handleDebug(htmlx, str, node); break; case 'InlineComponent': - if (usesLet(node)) { - templateScope.value = templateScope.value.child(); - } - handleComponent(htmlx, str, node, parent, ifScope, templateScope.value); + templateScopeManager.componentOrSlotTemplateOrElementEnter(node); + handleComponent( + htmlx, + str, + node, + parent, + ifScope, + templateScopeManager.value + ); break; case 'Element': - if (usesLet(node)) { - templateScope.value = templateScope.value.child(); - } - handleElement(htmlx, str, node, parent, ifScope, templateScope.value); + templateScopeManager.componentOrSlotTemplateOrElementEnter(node); + handleElement( + htmlx, + str, + node, + parent, + ifScope, + templateScopeManager.value + ); break; case 'Comment': handleComment(str, node); @@ -157,8 +146,8 @@ export function convertHtmlxToJsx( break; case 'SlotTemplate': handleSvelteTag(htmlx, str, node); + templateScopeManager.componentOrSlotTemplateOrElementEnter(node); if (usesLet(node)) { - templateScope.value = templateScope.value.child(); handleSlot( htmlx, str, @@ -166,7 +155,7 @@ export function convertHtmlxToJsx( parent, getSlotName(node) || 'default', ifScope, - templateScope.value + templateScopeManager.value ); } break; @@ -190,19 +179,15 @@ export function convertHtmlxToJsx( ifScope = ifScope.getParent(); break; case 'EachBlock': - if (!node.else) { - templateScope.value = templateScope.value.parent; - } + templateScopeManager.eachLeave(node); break; case 'AwaitBlock': - templateScope.value = templateScope.value.parent; + templateScopeManager.awaitLeave(); break; case 'InlineComponent': case 'Element': case 'SlotTemplate': - if (usesLet(node)) { - templateScope.value = templateScope.value.parent; - } + templateScopeManager.componentOrSlotTemplateOrElementLeave(node); break; } if (onLeave) { @@ -216,17 +201,6 @@ export function convertHtmlxToJsx( }); } -function handleScope(identifierDef: Node, templateScope: TemplateScope) { - if (isIdentifier(identifierDef)) { - templateScope.add(identifierDef.name); - } - if (isDestructuringPatterns(identifierDef)) { - // the node object is returned as-it with no mutation - const identifiers = extract_identifiers(identifierDef) as SvelteIdentifier[]; - templateScope.addMany(identifiers.map((id) => id.name)); - } -} - /** * @internal For testing only */ diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/component.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/component.ts index ed55e6c02..b98c53c0b 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/component.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/component.ts @@ -3,7 +3,7 @@ import { Node } from 'estree-walker'; import { getSlotName } from '../../utils/svelteAst'; import { handleSlot } from './slot'; import { IfScope } from './if-scope'; -import TemplateScope from '../nodes/template-scope'; +import { TemplateScope } from '../nodes/template-scope'; /** * Handle `` and slot-specific transformations. diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/element.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/element.ts index 6e6454274..120086b41 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/element.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/element.ts @@ -3,7 +3,7 @@ import { Node } from 'estree-walker'; import { getSlotName } from '../../utils/svelteAst'; import { handleSlot } from './slot'; import { IfScope } from './if-scope'; -import TemplateScope from '../nodes/template-scope'; +import { TemplateScope } from '../nodes/template-scope'; /** * Special treatment for self-closing / void tags to make them conform to JSX. diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts index ab6171103..933bfac57 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts @@ -1,7 +1,7 @@ import MagicString from 'magic-string'; import { Node } from 'estree-walker'; import { getIdentifiersInIfExpression } from '../utils/node-utils'; -import TemplateScope from '../nodes/template-scope'; +import { TemplateScope } from '../nodes/template-scope'; enum IfType { If, diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/slot.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/slot.ts index df446c208..fd5713903 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/slot.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/slot.ts @@ -3,7 +3,7 @@ import { Node } from 'estree-walker'; import { beforeStart } from '../utils/node-utils'; import { getSingleSlotDef } from '../../svelte2tsx/nodes/slot'; import { IfScope } from './if-scope'; -import TemplateScope from '../nodes/template-scope'; +import { TemplateScope } from '../nodes/template-scope'; export function handleSlot( htmlx: string, @@ -70,7 +70,3 @@ export function handleSlot( : slotEl.end; str.appendLeft(closeSlotDefInsertionPoint, `}}${constRedeclares ? '}' : ''}`); } - -export function usesLet(node: Node) { - return node.attributes?.some((attr) => attr.type === 'Let'); -} diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/template-scope.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/template-scope.ts index e97fb92bc..478b6c871 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/template-scope.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/template-scope.ts @@ -1,4 +1,10 @@ -export default class TemplateScope { +import { Node } from 'estree-walker'; +import { extract_identifiers } from 'periscopic'; +import { SvelteIdentifier } from '../../interfaces'; +import { isDestructuringPatterns, isIdentifier } from '../../utils/svelteAst'; +import { usesLet } from '../utils/node-utils'; + +export class TemplateScope { inits = new Set(); parent?: TemplateScope; @@ -21,3 +27,66 @@ export default class TemplateScope { return child; } } + +export class TemplateScopeManager { + value = new TemplateScope(); + + eachEnter(node: Node) { + this.value = this.value.child(); + if (node.context) { + this.handleScope(node.context); + } + if (node.index) { + this.value.add(node.index); + } + } + + eachLeave(node: Node) { + if (!node.else) { + this.value = this.value.parent; + } + } + + awaitEnter(node: Node) { + this.value = this.value.child(); + if (node.value) { + this.handleScope(node.value); + } + if (node.error) { + this.handleScope(node.error); + } + } + + awaitLeave() { + this.value = this.value.parent; + } + + elseEnter(parent: Node) { + if (parent.type === 'EachBlock') { + this.value = this.value.parent; + } + } + + componentOrSlotTemplateOrElementEnter(node: Node) { + if (usesLet(node)) { + this.value = this.value.child(); + } + } + + componentOrSlotTemplateOrElementLeave(node: Node) { + if (usesLet(node)) { + this.value = this.value.parent; + } + } + + private handleScope(identifierDef: Node) { + if (isIdentifier(identifierDef)) { + this.value.add(identifierDef.name); + } + if (isDestructuringPatterns(identifierDef)) { + // the node object is returned as-it with no mutation + const identifiers = extract_identifiers(identifierDef) as SvelteIdentifier[]; + this.value.addMany(identifiers.map((id) => id.name)); + } + } +} diff --git a/packages/svelte2tsx/src/htmlxtojsx/utils/node-utils.ts b/packages/svelte2tsx/src/htmlxtojsx/utils/node-utils.ts index 05c0c98dc..1a8e29c9b 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/utils/node-utils.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/utils/node-utils.ts @@ -63,3 +63,7 @@ export function getIdentifiersInIfExpression( return identifiers; } + +export function usesLet(node: Node): boolean { + return node.attributes?.some((attr) => attr.type === 'Let'); +} From 70a38bf8ebf7f0444a24d14dd476ad5d02901687 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 3 Mar 2021 12:03:19 +0100 Subject: [PATCH 08/17] remove comments --- .../svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts index 933bfac57..4fd83ab54 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts @@ -125,11 +125,6 @@ function collectReferencedIdentifiers(condition: Condition | undefined): Set Date: Wed, 3 Mar 2021 17:31:08 +0100 Subject: [PATCH 09/17] lint --- packages/svelte2tsx/src/htmlxtojsx/nodes/await.ts | 2 +- packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts | 4 ++-- packages/svelte2tsx/src/htmlxtojsx/utils/node-utils.ts | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/await.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/await.ts index 63cae1003..9cb2a842e 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/await.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/await.ts @@ -66,7 +66,7 @@ export function handleAwait( str.overwrite(thenStart, awaitBlock.value.start, '__sveltets_awaitThen(_$$p, ('); str.overwrite(awaitBlock.value.end, thenEnd, `) => {${ifScope.addPossibleIfCondition()}<>`); } else { - const awaitThenFn = `__sveltets_awaitThen(_$$p, () => {${ifScope.addPossibleIfCondition()}<>`; + const awaitThenFn = `__sveltets_awaitThen(_$$p, () => {${ifScope.addPossibleIfCondition()}<>`; // eslint-disable-line if (thenStart === thenEnd) { str.appendLeft(thenStart, awaitThenFn); } else { diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts index 4fd83ab54..d9fa93592 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts @@ -10,7 +10,7 @@ enum IfType { } interface ConditionInfo { - identifiers: Map; + identifiers: Map>; condition: string; } @@ -80,7 +80,7 @@ function getConditionString( replacedNames: string[], replacementPrefix: string ): string { - const replacements: { name: string; start: number; end: number }[] = []; + const replacements: Array<{ name: string; start: number; end: number }> = []; for (const name of replacedNames) { const occurences = condition.identifiers.get(name); if (occurences) { diff --git a/packages/svelte2tsx/src/htmlxtojsx/utils/node-utils.ts b/packages/svelte2tsx/src/htmlxtojsx/utils/node-utils.ts index 1a8e29c9b..7174f53d0 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/utils/node-utils.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/utils/node-utils.ts @@ -35,9 +35,9 @@ export function isQuote(str: string): boolean { export function getIdentifiersInIfExpression( expression: Node -): Map { +): Map> { const offset = expression.start; - const identifiers = new Map(); + const identifiers = new Map>(); walk(expression, { enter: (node, parent) => { switch (node.type) { From b258eb911c92ee4152d88f62db9bd55037944eea Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 3 Mar 2021 17:53:57 +0100 Subject: [PATCH 10/17] docs --- .../src/htmlxtojsx/nodes/if-scope.ts | 97 ++++++++++++++++--- 1 file changed, 82 insertions(+), 15 deletions(-) diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts index d9fa93592..8a13d2f52 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts @@ -11,7 +11,7 @@ enum IfType { interface ConditionInfo { identifiers: Map>; - condition: string; + text: string; } interface IfCondition { @@ -33,6 +33,10 @@ type Condition = IfCondition | ElseIfCondition | ElseCondition; const REPLACEMENT_PREFIX = '\u03A9'; +/** + * Returns the full currently known condition. Identifiers in the condition + * get replaced if they were redeclared. + */ function getFullCondition( condition: Condition, replacedNames: string[], @@ -75,6 +79,10 @@ function _getFullCondition( } } +/** + * Alter a condition text such that identifiers which needs replacement + * are replaced accordingly. + */ function getConditionString( condition: ConditionInfo, replacedNames: string[], @@ -91,23 +99,26 @@ function getConditionString( } if (!replacements.length) { - return condition.condition; + return condition.text; } replacements.sort((r1, r2) => r1.start - r2.start); return ( - condition.condition.substring(0, replacements[0].start) + + condition.text.substring(0, replacements[0].start) + replacements .map( (replacement, idx) => replacementPrefix + replacement.name + - condition.condition.substring(replacement.end, replacements[idx + 1]?.start) + condition.text.substring(replacement.end, replacements[idx + 1]?.start) ) .join('') ); } +/** + * Returns a set of all identifiers that were used in this condition + */ function collectReferencedIdentifiers(condition: Condition | undefined): Set { const identifiers = new Set(); let current = condition; @@ -125,6 +136,27 @@ function collectReferencedIdentifiers(condition: Condition | undefined): Set {

hi

}} : ''}` + * becomes + * `{check ? {() => {check &&

hi

}} : ''}` + * + * Most of the logic in here deals with the possibility of shadowed variables. + * Example: + * `{check ? {(check) => {

{check}

}} : ''}` + * becomes + * `{check ? {const Ωcheck = check;(check) => {Ωcheck &&

{check}

}} : ''}` + * + */ export class IfScope { private child?: IfScope; private ownScope = this.scope.value; @@ -136,6 +168,10 @@ export class IfScope { private parent?: IfScope ) {} + /** + * Returns the full currently known condition, prepended with the conditions + * of its parents. Identifiers in the condition get replaced if they were redeclared. + */ getFullCondition(): string { if (!this.current) { return ''; @@ -144,25 +180,35 @@ export class IfScope { const parentCondition = this.parent?.getFullCondition(); const condition = `(${getFullCondition( this.current, - this.getReplacedNames(), + this.getNamesThatNeedReplacement(), this.replacementPrefix )})`; return parentCondition ? `(${parentCondition}) && ${condition}` : condition; } + /** + * Convenience method which invokes `getFullCondition` and adds a `&&` at the end + * for easy chaining. + */ addPossibleIfCondition(): string { const condition = this.getFullCondition(); return condition ? `${condition} && ` : ''; } + /** + * Adds a new child IfScope. + */ addNestedIf(expression: Node, str: MagicString): void { - const condition = this.getConditionString(str, expression); + const condition = this.getConditionInfo(str, expression); const ifScope = new IfScope(this.scope, { condition, type: IfType.If }, this); this.child = ifScope; } + /** + * Adds a `else if` branch to the scope and enhances the condition accordingly. + */ addElseIf(expression: Node, str: MagicString): void { - const condition = this.getConditionString(str, expression); + const condition = this.getConditionInfo(str, expression); this.current = { condition, parent: this.current as IfCondition | ElseIfCondition, @@ -170,6 +216,9 @@ export class IfScope { }; } + /** + * Adds a `else` branch to the scope and enhances the condition accordingly. + */ addElse(): void { this.current = { parent: this.current as IfCondition | ElseIfCondition, type: IfType.Else }; } @@ -182,6 +231,9 @@ export class IfScope { return this.parent || this; } + /** + * Returns a set of all identifiers that were used in this IfScope and its parent scopes. + */ collectReferencedIdentifiers(): Set { const current = collectReferencedIdentifiers(this.current); const parent = this.parent?.collectReferencedIdentifiers(); @@ -193,6 +245,12 @@ export class IfScope { return current; } + /** + * Should be invoked when a new template scope which resets control flow (await, each, slot) is created. + * The returned string contains a list of `const` declarations which redeclares the identifiers + * in the conditions which would be overwritten by the scope + * (because they declare a variable with the same name, therefore shadowing the outer variable). + */ getConstsToRedeclare(): string { const replacements = this.getNamesToRedeclare() .map((identifier) => `${this.replacementPrefix + identifier}=${identifier}`) @@ -200,10 +258,9 @@ export class IfScope { return replacements ? `const ${replacements};` : ''; } - addConstsSuffixIfNecessary(): string { - return this.getNamesToRedeclare().length ? '}' : ''; - } - + /** + * Returns true if given identifier is referenced in this IfScope or a parent scope. + */ referencesIdentifier(name: string): boolean { const current = collectReferencedIdentifiers(this.current); if (current.has(name)) { @@ -215,12 +272,15 @@ export class IfScope { return this.parent.referencesIdentifier(name); } - private getConditionString(str: MagicString, expression: Node) { + private getConditionInfo(str: MagicString, expression: Node) { const identifiers = getIdentifiersInIfExpression(expression); - const condition = str.original.substring(expression.start, expression.end); - return { identifiers, condition }; + const text = str.original.substring(expression.start, expression.end); + return { identifiers, text }; } + /** + * Contains a list of identifiers which would be overwritten by the child template scope. + */ private getNamesToRedeclare() { return [...this.scope.value.inits.keys()].filter((init) => { let parent = this.scope.value.parent; @@ -234,13 +294,20 @@ export class IfScope { }); } - private getReplacedNames() { + /** + * Return all identifiers that were redeclared and therefore need replacement. + */ + private getNamesThatNeedReplacement() { const referencedIdentifiers = this.collectReferencedIdentifiers(); return [...referencedIdentifiers].filter((identifier) => this.someChildScopeHasRedeclaredVariable(identifier) ); } + /** + * Returns true if given identifier name is redeclared in a child template scope + * and is therefore shadowed within that scope. + */ private someChildScopeHasRedeclaredVariable(name: string) { let scope = this.scope.value; while (scope && scope !== this.ownScope) { From 78c6930325f73374c425ef21fffadea7570ca0b0 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 4 Mar 2021 10:26:40 +0100 Subject: [PATCH 11/17] fix await --- .../svelte2tsx/src/htmlxtojsx/nodes/await.ts | 4 ++-- .../src/htmlxtojsx/nodes/if-scope.ts | 22 +++++++++++-------- .../expected.jsx | 15 ++++++++++++- .../input.svelte | 13 +++++++++++ .../if-nested-await-block/expected.jsx | 3 +++ .../if-nested-await-block/input.svelte | 3 +++ 6 files changed, 48 insertions(+), 12 deletions(-) diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/await.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/await.ts index 9cb2a842e..b7ee1d2b7 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/await.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/await.ts @@ -41,7 +41,7 @@ export function handleAwait( // somePromise} -> somePromise); str.overwrite(awaitBlock.expression.end, awaitEnd + 1, ');'); - str.appendRight(awaitEnd + 1, ` ${ifScope.addPossibleIfCondition()}<>`); + str.appendRight(awaitEnd + 1, ` ${ifScope.addPossibleIfCondition(true)}<>`); } else { // {await ... then ...} thenStart = htmlx.indexOf('then', awaitBlock.expression.end); @@ -58,7 +58,7 @@ export function handleAwait( const awaitEnd = htmlx.indexOf('}', awaitBlock.expression.end); str.overwrite(awaitBlock.expression.end, awaitEnd + 1, ');'); - str.appendRight(awaitEnd + 1, ' <>'); + str.appendRight(awaitEnd + 1, ` ${ifScope.addPossibleIfCondition(true)}<>`); str.appendLeft(thenEnd, '; '); } diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts index 8a13d2f52..e640b06c3 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts @@ -172,15 +172,15 @@ export class IfScope { * Returns the full currently known condition, prepended with the conditions * of its parents. Identifiers in the condition get replaced if they were redeclared. */ - getFullCondition(): string { + getFullCondition(skipImmediateChildScope = false): string { if (!this.current) { return ''; } - const parentCondition = this.parent?.getFullCondition(); + const parentCondition = this.parent?.getFullCondition(false); const condition = `(${getFullCondition( this.current, - this.getNamesThatNeedReplacement(), + this.getNamesThatNeedReplacement(skipImmediateChildScope), this.replacementPrefix )})`; return parentCondition ? `(${parentCondition}) && ${condition}` : condition; @@ -190,8 +190,8 @@ export class IfScope { * Convenience method which invokes `getFullCondition` and adds a `&&` at the end * for easy chaining. */ - addPossibleIfCondition(): string { - const condition = this.getFullCondition(); + addPossibleIfCondition(skipImmediateChildScope = false): string { + const condition = this.getFullCondition(skipImmediateChildScope); return condition ? `${condition} && ` : ''; } @@ -297,10 +297,10 @@ export class IfScope { /** * Return all identifiers that were redeclared and therefore need replacement. */ - private getNamesThatNeedReplacement() { + private getNamesThatNeedReplacement(skipImmediateChildScope: boolean) { const referencedIdentifiers = this.collectReferencedIdentifiers(); return [...referencedIdentifiers].filter((identifier) => - this.someChildScopeHasRedeclaredVariable(identifier) + this.someChildScopeHasRedeclaredVariable(identifier, skipImmediateChildScope) ); } @@ -308,9 +308,13 @@ export class IfScope { * Returns true if given identifier name is redeclared in a child template scope * and is therefore shadowed within that scope. */ - private someChildScopeHasRedeclaredVariable(name: string) { + private someChildScopeHasRedeclaredVariable(name: string, skipImmediateChildScope: boolean) { let scope = this.scope.value; - while (scope && scope !== this.ownScope) { + while ( + scope && + (!skipImmediateChildScope || scope.parent !== this.ownScope) && + scope !== this.ownScope + ) { if (scope.inits.has(name)) { return true; } diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/expected.jsx b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/expected.jsx index cb74c9fa8..a1b211c8f 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/expected.jsx +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/expected.jsx @@ -2,6 +2,14 @@ {() => {const Ωhello=hello;let _$$p = (hello); __sveltets_awaitThen(_$$p, (hello) => {((Ωhello)) && <> {hello} {(hello) ? <> + {() => {let _$$p = (aPromise); (((Ωhello))) && ((hello)) && <> + {hello} + ; __sveltets_awaitThen(_$$p, () => {(((Ωhello))) && ((hello)) && <>})}} + {() => {const ΩΩhello=hello;let _$$p = (aPromise); (((Ωhello))) && ((hello)) && <> + {hello} + ; __sveltets_awaitThen(_$$p, () => {(((Ωhello))) && ((ΩΩhello)) && <>}, (hello) => {(((Ωhello))) && ((ΩΩhello)) && <> + {hello} + })}} {() => {const ΩΩhello=hello;let _$$p = (x); __sveltets_awaitThen(_$$p, (hello) => {(((Ωhello))) && ((ΩΩhello)) && <> {(hello) ? <> {hello} @@ -18,7 +26,7 @@ : <>} })}} : (cool) ? <> - {() => {const Ωcool=cool;let _$$p = (cool); (((hello))) && (!(hi && bye) && (Ωcool)) && <> + {() => {const Ωcool=cool;let _$$p = (cool); (((hello))) && (!(hi && bye) && (cool)) && <> loading ; __sveltets_awaitThen(_$$p, (cool) => {(((hello))) && (!(hi && bye) && (Ωcool)) && <> {(cool) ? <> @@ -27,6 +35,11 @@ }, (cool) => {(((hello))) && (!(hi && bye) && (Ωcool)) && <> z })}} + {() => {const Ωcool=cool;let _$$p = (aPromise); (((hello))) && (!(hi && bye) && (cool)) && <> + loading + ; __sveltets_awaitThen(_$$p, (cool) => {(((hello))) && (!(hi && bye) && (Ωcool)) && <> + {cool} + })}} : <> {() => {const Ωhello=hello;let _$$p = (x); __sveltets_awaitThen(_$$p, (hello) => {(((Ωhello))) && (!(hi && bye) && !(cool)) && <> {(hello) ? <> diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/input.svelte b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/input.svelte index 116faa090..0b5c801c3 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/input.svelte +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/input.svelte @@ -2,6 +2,14 @@ {#await hello then hello} {hello} {#if hello} + {#await aPromise} + {hello} + {/await} + {#await aPromise} + {hello} + {:catch hello} + {hello} + {/await} {#await x then hello} {#if hello} {hello} @@ -27,6 +35,11 @@ {:catch cool} z {/await} + {#await aPromise} + loading + {:then cool} + {cool} + {/await} {:else} {#await x then hello} {#if hello} diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/expected.jsx b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/expected.jsx index 2ce1c2950..37e34066a 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/expected.jsx +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/expected.jsx @@ -2,6 +2,9 @@ {() => {let _$$p = (x); __sveltets_awaitThen(_$$p, (y) => {((hello)) && <> {y} })}} + {() => {let _$$p = (aPromise); ((hello)) && <> + {hello} + ; __sveltets_awaitThen(_$$p, () => {((hello)) && <>})}} {(hi && bye) ? <> {() => {let _$$p = (x); __sveltets_awaitThen(_$$p, (y) => {(((hello))) && ((hi && bye)) && <> {y} diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/input.svelte b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/input.svelte index f5e6f03f4..dfc6c93b1 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/input.svelte +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/input.svelte @@ -2,6 +2,9 @@ {#await x then y} {y} {/await} + {#await aPromise} + {hello} + {/await} {#if hi && bye} {#await x then y} {y} From b641d6a083482267799d3cca5005bf6c8df582e8 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 4 Mar 2021 10:26:53 +0100 Subject: [PATCH 12/17] diagnostics test --- .../features/DiagnosticsProvider.test.ts | 96 +++++++++++++++++++ ...iagnostics-if-control-flow-imported.svelte | 2 +- ...iagnostics-if-control-flow-shadowed.svelte | 46 +++++++++ 3 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 packages/language-server/test/plugins/typescript/testfiles/diagnostics/diagnostics-if-control-flow-shadowed.svelte diff --git a/packages/language-server/test/plugins/typescript/features/DiagnosticsProvider.test.ts b/packages/language-server/test/plugins/typescript/features/DiagnosticsProvider.test.ts index 2475d8258..1e3a09a16 100644 --- a/packages/language-server/test/plugins/typescript/features/DiagnosticsProvider.test.ts +++ b/packages/language-server/test/plugins/typescript/features/DiagnosticsProvider.test.ts @@ -610,4 +610,100 @@ describe('DiagnosticsProvider', () => { } ]); }); + + it('if control flow with shadowed variables', async () => { + const { plugin, document } = setup('diagnostics-if-control-flow-shadowed.svelte'); + const diagnostics = await plugin.getDiagnostics(document); + + assert.deepStrictEqual(diagnostics, [ + { + code: 2367, + message: + "This condition will always return 'false' since the types 'string' and 'boolean' have no overlap.", + range: { + end: { + character: 15, + line: 13 + }, + start: { + character: 5, + line: 13 + } + }, + severity: 1, + source: 'ts', + tags: [] + }, + { + code: 2322, + message: "Type 'boolean' is not assignable to type 'string'.", + range: { + end: { + character: 16, + line: 17 + }, + start: { + character: 9, + line: 17 + } + }, + severity: 1, + source: 'ts', + tags: [] + }, + { + code: 2339, + message: "Property 'a' does not exist on type 'boolean'.", + range: { + end: { + character: 16, + line: 23 + }, + start: { + character: 15, + line: 23 + } + }, + severity: 1, + source: 'ts', + tags: [] + }, + { + code: 2339, + message: + "Property 'a' does not exist on type 'string | boolean'.\n Property 'a' does not exist on type 'string'.", + range: { + end: { + character: 16, + line: 29 + }, + start: { + character: 15, + line: 29 + } + }, + severity: 1, + source: 'ts', + tags: [] + }, + { + code: 2367, + message: + "This condition will always return 'false' since the types 'string' and 'boolean' have no overlap.", + range: { + end: { + character: 24, + line: 31 + }, + start: { + character: 17, + line: 31 + } + }, + severity: 1, + source: 'ts', + tags: [] + } + ]); + }); }); diff --git a/packages/language-server/test/plugins/typescript/testfiles/diagnostics/diagnostics-if-control-flow-imported.svelte b/packages/language-server/test/plugins/typescript/testfiles/diagnostics/diagnostics-if-control-flow-imported.svelte index 794223e78..8da30703a 100644 --- a/packages/language-server/test/plugins/typescript/testfiles/diagnostics/diagnostics-if-control-flow-imported.svelte +++ b/packages/language-server/test/plugins/typescript/testfiles/diagnostics/diagnostics-if-control-flow-imported.svelte @@ -2,4 +2,4 @@ const foo: string | boolean = '' as any; - + diff --git a/packages/language-server/test/plugins/typescript/testfiles/diagnostics/diagnostics-if-control-flow-shadowed.svelte b/packages/language-server/test/plugins/typescript/testfiles/diagnostics/diagnostics-if-control-flow-shadowed.svelte new file mode 100644 index 000000000..3e8000d1a --- /dev/null +++ b/packages/language-server/test/plugins/typescript/testfiles/diagnostics/diagnostics-if-control-flow-shadowed.svelte @@ -0,0 +1,46 @@ + + +{#if typeof a === 'string'} + {a === true} + {assignA = a} + {#each [true] as a} + {a === true} + {assignA = a} + {/each} + {#if b} + {#await aPromise} + {b.a} + {:then b} + {b.a} + {/await} + {b.a} + {:else} + {b === true} + + {b.a} + {#if typeof b === 'boolean'} + {a === b} + {:else} + {a === b} + {/if} + + {/if} +{:else} + {#if typeof $store === 'string'} + {#each [] as a} + {$store === a} + {/each} + {:else} + {$store === a} + {/if} +{/if} \ No newline at end of file From 68dc2a7037c1572cc8cf5c6af64cdf5888ce71e8 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 4 Mar 2021 11:18:25 +0100 Subject: [PATCH 13/17] add generated comment. use it to filter out diagnostics --- .../features/DiagnosticsProvider.ts | 23 +++++- .../src/plugins/typescript/features/utils.ts | 13 +++ .../features/DiagnosticsProvider.test.ts | 80 +++++++++++++++++++ .../diagnostics-ignore-generated.svelte | 12 +++ .../src/htmlxtojsx/nodes/if-scope.ts | 5 +- packages/svelte2tsx/src/utils/ignore.ts | 10 +++ .../expected.jsx | 28 +++---- .../if-nested-await-block/expected.jsx | 18 ++--- .../expected.jsx | 10 +-- .../samples/if-nested-each-block/expected.jsx | 8 +- .../if-nested-slot-let-shadowed/expected.jsx | 18 ++--- .../samples/if-nested-slot-let/expected.jsx | 8 +- .../expected.tsx | 4 +- 13 files changed, 187 insertions(+), 50 deletions(-) create mode 100644 packages/language-server/test/plugins/typescript/testfiles/diagnostics/diagnostics-ignore-generated.svelte create mode 100644 packages/svelte2tsx/src/utils/ignore.ts diff --git a/packages/language-server/src/plugins/typescript/features/DiagnosticsProvider.ts b/packages/language-server/src/plugins/typescript/features/DiagnosticsProvider.ts index 50194860a..c8140bf63 100644 --- a/packages/language-server/src/plugins/typescript/features/DiagnosticsProvider.ts +++ b/packages/language-server/src/plugins/typescript/features/DiagnosticsProvider.ts @@ -1,10 +1,17 @@ import ts from 'typescript'; import { Diagnostic, DiagnosticSeverity, DiagnosticTag } from 'vscode-languageserver'; -import { Document, mapObjWithRangeToOriginal, getTextInRange } from '../../../lib/documents'; +import { + Document, + mapObjWithRangeToOriginal, + getTextInRange, + getLineAtPosition, + offsetAt +} from '../../../lib/documents'; import { DiagnosticsProvider } from '../../interfaces'; import { LSAndTSDocResolver } from '../LSAndTSDocResolver'; import { convertRange, mapSeverity } from '../utils'; import { SvelteDocumentSnapshot } from '../DocumentSnapshot'; +import { isInGeneratedCode } from './utils'; export class DiagnosticsProviderImpl implements DiagnosticsProvider { constructor(private readonly lsAndTsDocResolver: LSAndTSDocResolver) {} @@ -40,6 +47,7 @@ export class DiagnosticsProviderImpl implements DiagnosticsProvider { const fragment = await tsDoc.getFragment(); return diagnostics + .filter(isNotGenerated(tsDoc.getText(0, tsDoc.getLength()))) .map((diagnostic) => ({ range: convertRange(tsDoc, diagnostic), severity: mapSeverity(diagnostic.category), @@ -208,3 +216,16 @@ function swapRangeStartEndIfNecessary(diag: Diagnostic): Diagnostic { } return diag; } + +/** + * Checks if diagnostic is not within a section that should be completely ignored + * because it's purely generated. + */ +function isNotGenerated(text: string) { + return (diagnostic: ts.Diagnostic) => { + if (diagnostic.start === undefined || diagnostic.length === undefined) { + return true; + } + return !isInGeneratedCode(text, diagnostic.start, diagnostic.start + diagnostic.length); + }; +} diff --git a/packages/language-server/src/plugins/typescript/features/utils.ts b/packages/language-server/src/plugins/typescript/features/utils.ts index afefc9e60..d06895fc4 100644 --- a/packages/language-server/src/plugins/typescript/features/utils.ts +++ b/packages/language-server/src/plugins/typescript/features/utils.ts @@ -48,3 +48,16 @@ export function getComponentAtPosition( } return snapshot; } + +/** + * Checks if this a section that should be completely ignored + * because it's purely generated. + */ +export function isInGeneratedCode(text: string, start: number, end: number) { + const lineStart = text.lastIndexOf('\n', start); + const lineEnd = text.indexOf('\n', end); + return ( + text.substring(lineStart, start).includes('/*Ωignore_startΩ*/') && + text.substring(end, lineEnd).includes('/*Ωignore_endΩ*/') + ); +} diff --git a/packages/language-server/test/plugins/typescript/features/DiagnosticsProvider.test.ts b/packages/language-server/test/plugins/typescript/features/DiagnosticsProvider.test.ts index 1e3a09a16..253e06739 100644 --- a/packages/language-server/test/plugins/typescript/features/DiagnosticsProvider.test.ts +++ b/packages/language-server/test/plugins/typescript/features/DiagnosticsProvider.test.ts @@ -706,4 +706,84 @@ describe('DiagnosticsProvider', () => { } ]); }); + + it('ignores diagnostics in generated code', async () => { + const { plugin, document } = setup('diagnostics-ignore-generated.svelte'); + const diagnostics = await plugin.getDiagnostics(document); + + assert.deepStrictEqual( + diagnostics, + + [ + { + code: 2304, + message: "Cannot find name 'a'.", + range: { + end: { + character: 13, + line: 3 + }, + start: { + character: 12, + line: 3 + } + }, + severity: 1, + source: 'ts', + tags: [] + }, + { + code: 2304, + message: "Cannot find name 'a'.", + range: { + end: { + character: 6, + line: 4 + }, + start: { + character: 5, + line: 4 + } + }, + severity: 1, + source: 'ts', + tags: [] + }, + { + code: 2304, + message: "Cannot find name 'b'.", + range: { + end: { + character: 10, + line: 8 + }, + start: { + character: 9, + line: 8 + } + }, + severity: 1, + source: 'ts', + tags: [] + }, + { + code: 2304, + message: "Cannot find name 'b'.", + range: { + end: { + character: 10, + line: 9 + }, + start: { + character: 9, + line: 9 + } + }, + severity: 1, + source: 'ts', + tags: [] + } + ] + ); + }); }); diff --git a/packages/language-server/test/plugins/typescript/testfiles/diagnostics/diagnostics-ignore-generated.svelte b/packages/language-server/test/plugins/typescript/testfiles/diagnostics/diagnostics-ignore-generated.svelte new file mode 100644 index 000000000..1441cf0f8 --- /dev/null +++ b/packages/language-server/test/plugins/typescript/testfiles/diagnostics/diagnostics-ignore-generated.svelte @@ -0,0 +1,12 @@ + + +{#if typeof a === 'string'} + {a === true} + {#each [true] as a} + {a === true} + {/each} + {#if b} + {b} + {/if} +{/if} diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts index e640b06c3..778bf09ab 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts @@ -2,6 +2,7 @@ import MagicString from 'magic-string'; import { Node } from 'estree-walker'; import { getIdentifiersInIfExpression } from '../utils/node-utils'; import { TemplateScope } from '../nodes/template-scope'; +import { surroundWithIgnoreComments } from '../../utils/ignore'; enum IfType { If, @@ -192,7 +193,7 @@ export class IfScope { */ addPossibleIfCondition(skipImmediateChildScope = false): string { const condition = this.getFullCondition(skipImmediateChildScope); - return condition ? `${condition} && ` : ''; + return condition ? surroundWithIgnoreComments(`${condition} && `) : ''; } /** @@ -255,7 +256,7 @@ export class IfScope { const replacements = this.getNamesToRedeclare() .map((identifier) => `${this.replacementPrefix + identifier}=${identifier}`) .join(','); - return replacements ? `const ${replacements};` : ''; + return replacements ? surroundWithIgnoreComments(`const ${replacements};`) : ''; } /** diff --git a/packages/svelte2tsx/src/utils/ignore.ts b/packages/svelte2tsx/src/utils/ignore.ts new file mode 100644 index 000000000..81a370fc3 --- /dev/null +++ b/packages/svelte2tsx/src/utils/ignore.ts @@ -0,0 +1,10 @@ +export const IGNORE_START_COMMENT = '/*Ωignore_startΩ*/'; +export const IGNORE_END_COMMENT = '/*Ωignore_endΩ*/'; + +/** + * Surrounds given string with a start/end comment which marks it + * to be ignored by tooling. + */ +export function surroundWithIgnoreComments(str: string): string { + return IGNORE_START_COMMENT + str + IGNORE_END_COMMENT; +} diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/expected.jsx b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/expected.jsx index a1b211c8f..0cd41cc11 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/expected.jsx +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/expected.jsx @@ -1,16 +1,16 @@ <>{(hello) ? <> - {() => {const Ωhello=hello;let _$$p = (hello); __sveltets_awaitThen(_$$p, (hello) => {((Ωhello)) && <> + {() => {/*Ωignore_startΩ*/const Ωhello=hello;/*Ωignore_endΩ*/let _$$p = (hello); __sveltets_awaitThen(_$$p, (hello) => {/*Ωignore_startΩ*/((Ωhello)) && /*Ωignore_endΩ*/<> {hello} {(hello) ? <> - {() => {let _$$p = (aPromise); (((Ωhello))) && ((hello)) && <> + {() => {let _$$p = (aPromise); /*Ωignore_startΩ*/(((Ωhello))) && ((hello)) && /*Ωignore_endΩ*/<> {hello} - ; __sveltets_awaitThen(_$$p, () => {(((Ωhello))) && ((hello)) && <>})}} - {() => {const ΩΩhello=hello;let _$$p = (aPromise); (((Ωhello))) && ((hello)) && <> + ; __sveltets_awaitThen(_$$p, () => {/*Ωignore_startΩ*/(((Ωhello))) && ((hello)) && /*Ωignore_endΩ*/<>})}} + {() => {/*Ωignore_startΩ*/const ΩΩhello=hello;/*Ωignore_endΩ*/let _$$p = (aPromise); /*Ωignore_startΩ*/(((Ωhello))) && ((hello)) && /*Ωignore_endΩ*/<> {hello} - ; __sveltets_awaitThen(_$$p, () => {(((Ωhello))) && ((ΩΩhello)) && <>}, (hello) => {(((Ωhello))) && ((ΩΩhello)) && <> + ; __sveltets_awaitThen(_$$p, () => {/*Ωignore_startΩ*/(((Ωhello))) && ((ΩΩhello)) && /*Ωignore_endΩ*/<>}, (hello) => {/*Ωignore_startΩ*/(((Ωhello))) && ((ΩΩhello)) && /*Ωignore_endΩ*/<> {hello} })}} - {() => {const ΩΩhello=hello;let _$$p = (x); __sveltets_awaitThen(_$$p, (hello) => {(((Ωhello))) && ((ΩΩhello)) && <> + {() => {/*Ωignore_startΩ*/const ΩΩhello=hello;/*Ωignore_endΩ*/let _$$p = (x); __sveltets_awaitThen(_$$p, (hello) => {/*Ωignore_startΩ*/(((Ωhello))) && ((ΩΩhello)) && /*Ωignore_endΩ*/<> {(hello) ? <> {hello} : <>} @@ -18,30 +18,30 @@ : <>} })}} {(hi && bye) ? <> - {() => {const Ωbye=bye,Ωhello=hello;let _$$p = (x); __sveltets_awaitThen(_$$p, (bye) => {(((Ωhello))) && ((hi && Ωbye)) && <> + {() => {/*Ωignore_startΩ*/const Ωbye=bye,Ωhello=hello;/*Ωignore_endΩ*/let _$$p = (x); __sveltets_awaitThen(_$$p, (bye) => {/*Ωignore_startΩ*/(((Ωhello))) && ((hi && Ωbye)) && /*Ωignore_endΩ*/<> {bye} - }, (hello) => {(((Ωhello))) && ((hi && Ωbye)) && <> + }, (hello) => {/*Ωignore_startΩ*/(((Ωhello))) && ((hi && Ωbye)) && /*Ωignore_endΩ*/<> {(hello) ? <> {hello} : <>} })}} : (cool) ? <> - {() => {const Ωcool=cool;let _$$p = (cool); (((hello))) && (!(hi && bye) && (cool)) && <> + {() => {/*Ωignore_startΩ*/const Ωcool=cool;/*Ωignore_endΩ*/let _$$p = (cool); /*Ωignore_startΩ*/(((hello))) && (!(hi && bye) && (cool)) && /*Ωignore_endΩ*/<> loading - ; __sveltets_awaitThen(_$$p, (cool) => {(((hello))) && (!(hi && bye) && (Ωcool)) && <> + ; __sveltets_awaitThen(_$$p, (cool) => {/*Ωignore_startΩ*/(((hello))) && (!(hi && bye) && (Ωcool)) && /*Ωignore_endΩ*/<> {(cool) ? <> {cool} : <>} - }, (cool) => {(((hello))) && (!(hi && bye) && (Ωcool)) && <> + }, (cool) => {/*Ωignore_startΩ*/(((hello))) && (!(hi && bye) && (Ωcool)) && /*Ωignore_endΩ*/<> z })}} - {() => {const Ωcool=cool;let _$$p = (aPromise); (((hello))) && (!(hi && bye) && (cool)) && <> + {() => {/*Ωignore_startΩ*/const Ωcool=cool;/*Ωignore_endΩ*/let _$$p = (aPromise); /*Ωignore_startΩ*/(((hello))) && (!(hi && bye) && (cool)) && /*Ωignore_endΩ*/<> loading - ; __sveltets_awaitThen(_$$p, (cool) => {(((hello))) && (!(hi && bye) && (Ωcool)) && <> + ; __sveltets_awaitThen(_$$p, (cool) => {/*Ωignore_startΩ*/(((hello))) && (!(hi && bye) && (Ωcool)) && /*Ωignore_endΩ*/<> {cool} })}} : <> - {() => {const Ωhello=hello;let _$$p = (x); __sveltets_awaitThen(_$$p, (hello) => {(((Ωhello))) && (!(hi && bye) && !(cool)) && <> + {() => {/*Ωignore_startΩ*/const Ωhello=hello;/*Ωignore_endΩ*/let _$$p = (x); __sveltets_awaitThen(_$$p, (hello) => {/*Ωignore_startΩ*/(((Ωhello))) && (!(hi && bye) && !(cool)) && /*Ωignore_endΩ*/<> {(hello) ? <> {hello} : <>} diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/expected.jsx b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/expected.jsx index 37e34066a..934fa1806 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/expected.jsx +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/expected.jsx @@ -1,26 +1,26 @@ <>{(hello) ? <> - {() => {let _$$p = (x); __sveltets_awaitThen(_$$p, (y) => {((hello)) && <> + {() => {let _$$p = (x); __sveltets_awaitThen(_$$p, (y) => {/*Ωignore_startΩ*/((hello)) && /*Ωignore_endΩ*/<> {y} })}} - {() => {let _$$p = (aPromise); ((hello)) && <> + {() => {let _$$p = (aPromise); /*Ωignore_startΩ*/((hello)) && /*Ωignore_endΩ*/<> {hello} - ; __sveltets_awaitThen(_$$p, () => {((hello)) && <>})}} + ; __sveltets_awaitThen(_$$p, () => {/*Ωignore_startΩ*/((hello)) && /*Ωignore_endΩ*/<>})}} {(hi && bye) ? <> - {() => {let _$$p = (x); __sveltets_awaitThen(_$$p, (y) => {(((hello))) && ((hi && bye)) && <> + {() => {let _$$p = (x); __sveltets_awaitThen(_$$p, (y) => {/*Ωignore_startΩ*/(((hello))) && ((hi && bye)) && /*Ωignore_endΩ*/<> {y} - }, () => {(((hello))) && ((hi && bye)) && <> + }, () => {/*Ωignore_startΩ*/(((hello))) && ((hi && bye)) && /*Ωignore_endΩ*/<> z })}} : (cool) ? <> - {() => {let _$$p = (x); (((hello))) && (!(hi && bye) && (cool)) && <> + {() => {let _$$p = (x); /*Ωignore_startΩ*/(((hello))) && (!(hi && bye) && (cool)) && /*Ωignore_endΩ*/<> loading - ; __sveltets_awaitThen(_$$p, (y) => {(((hello))) && (!(hi && bye) && (cool)) && <> + ; __sveltets_awaitThen(_$$p, (y) => {/*Ωignore_startΩ*/(((hello))) && (!(hi && bye) && (cool)) && /*Ωignore_endΩ*/<> {y} - }, () => {(((hello))) && (!(hi && bye) && (cool)) && <> + }, () => {/*Ωignore_startΩ*/(((hello))) && (!(hi && bye) && (cool)) && /*Ωignore_endΩ*/<> z })}} : <> - {() => {let _$$p = (x); __sveltets_awaitThen(_$$p, (y) => {(((hello))) && (!(hi && bye) && !(cool)) && <> + {() => {let _$$p = (x); __sveltets_awaitThen(_$$p, (y) => {/*Ωignore_startΩ*/(((hello))) && (!(hi && bye) && !(cool)) && /*Ωignore_endΩ*/<> {y} })}} } diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/expected.jsx b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/expected.jsx index e1760eb75..240661761 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/expected.jsx +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block-shadowed/expected.jsx @@ -1,8 +1,8 @@ <>{(hello) ? <> - {() => {const Ωhello=hello;() => {__sveltets_each(items, (hello,i) => (hello.id) && ((Ωhello)) && <> + {() => {/*Ωignore_startΩ*/const Ωhello=hello;/*Ωignore_endΩ*/() => {__sveltets_each(items, (hello,i) => (hello.id) && /*Ωignore_startΩ*/((Ωhello)) && /*Ωignore_endΩ*/<>
{hello}{i}
{(hello) ? <> - {() => {const ΩΩhello=hello;() => {__sveltets_each(items, (hello) => (((Ωhello))) && ((ΩΩhello)) && <> + {() => {/*Ωignore_startΩ*/const ΩΩhello=hello;/*Ωignore_endΩ*/() => {__sveltets_each(items, (hello) => /*Ωignore_startΩ*/(((Ωhello))) && ((ΩΩhello)) && /*Ωignore_endΩ*/<> {(hello) ? <> {hello} : <>} @@ -14,7 +14,7 @@ : <>} {(hi && bye) ? <> - {() => {const Ωbye=bye;() => {__sveltets_each(items, (bye) => (((hello))) && ((hi && Ωbye)) && <> + {() => {/*Ωignore_startΩ*/const Ωbye=bye;/*Ωignore_endΩ*/() => {__sveltets_each(items, (bye) => /*Ωignore_startΩ*/(((hello))) && ((hi && Ωbye)) && /*Ωignore_endΩ*/<>
{bye}
)}}} {(bye) ? <> @@ -22,11 +22,11 @@ : <>} : (cool) ? <> - {() => {const Ωcool=cool;() => {__sveltets_each(items, (item,cool) => (((hello))) && (!(hi && bye) && (Ωcool)) && <> + {() => {/*Ωignore_startΩ*/const Ωcool=cool;/*Ωignore_endΩ*/() => {__sveltets_each(items, (item,cool) => /*Ωignore_startΩ*/(((hello))) && (!(hi && bye) && (Ωcool)) && /*Ωignore_endΩ*/<>
{item}{cool}
)}}} : <> - {() => {const Ωhello=hello;() => {__sveltets_each(items, (hello) => (((Ωhello))) && (!(hi && bye) && !(cool)) && <> + {() => {/*Ωignore_startΩ*/const Ωhello=hello;/*Ωignore_endΩ*/() => {__sveltets_each(items, (hello) => /*Ωignore_startΩ*/(((Ωhello))) && (!(hi && bye) && !(cool)) && /*Ωignore_endΩ*/<>
{hello}
)}}} } diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block/expected.jsx b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block/expected.jsx index 4b531db5b..f9e7204fd 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block/expected.jsx +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-each-block/expected.jsx @@ -1,19 +1,19 @@ <>{(hello) ? <> - {__sveltets_each(items, (item,i) => (item.id) && ((hello)) && <> + {__sveltets_each(items, (item,i) => (item.id) && /*Ωignore_startΩ*/((hello)) && /*Ωignore_endΩ*/<>
{item}{i}
)} {(hi && bye) ? <> - {__sveltets_each(items, (item) => (((hello))) && ((hi && bye)) && <> + {__sveltets_each(items, (item) => /*Ωignore_startΩ*/(((hello))) && ((hi && bye)) && /*Ωignore_endΩ*/<>
{item}
)}

hi

: (cool) ? <> - {__sveltets_each(items, (item,i) => (((hello))) && (!(hi && bye) && (cool)) && <> + {__sveltets_each(items, (item,i) => /*Ωignore_startΩ*/(((hello))) && (!(hi && bye) && (cool)) && /*Ωignore_endΩ*/<>
{item}{i}
)} : <> - {__sveltets_each(items, (item) => (((hello))) && (!(hi && bye) && !(cool)) && <> + {__sveltets_each(items, (item) => /*Ωignore_startΩ*/(((hello))) && (!(hi && bye) && !(cool)) && /*Ωignore_endΩ*/<>
{item}
)} } diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed/expected.jsx b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed/expected.jsx index b77eebc42..65d1fd704 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed/expected.jsx +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let-shadowed/expected.jsx @@ -1,28 +1,28 @@ <>{(hello && hello1) ? <> - {() => {const Ωhello=hello;() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['default'];((Ωhello && hello1)) && <> + {() => {/*Ωignore_startΩ*/const Ωhello=hello;/*Ωignore_endΩ*/() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['default'];/*Ωignore_startΩ*/((Ωhello && hello1)) && /*Ωignore_endΩ*/<> {hello} - {() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['default'];((Ωhello && hello1)) && <> + {() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['default'];/*Ωignore_startΩ*/((Ωhello && hello1)) && /*Ωignore_endΩ*/<> {(hello) ? <> {hello} : <>} }} - {() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['named1'];((Ωhello && hello1)) && <> + {() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['named1'];/*Ωignore_startΩ*/((Ωhello && hello1)) && /*Ωignore_endΩ*/<> {(hello) ? <> {hello} : <>} }} - {() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['named2'];((Ωhello && hello1)) && <>

+ {() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['named2'];/*Ωignore_startΩ*/((Ωhello && hello1)) && /*Ωignore_endΩ*/<>

{(hello) ? <> {hello} : <>}

}} - {() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['named3'];((Ωhello && hello1)) && <> + {() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['named3'];/*Ωignore_startΩ*/((Ωhello && hello1)) && /*Ωignore_endΩ*/<> {(hello) ? <> {hello} : <>} }} {(hello) ? <> - {() => {const ΩΩhello=hello;() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['default'];(((Ωhello && hello1))) && ((ΩΩhello)) && <> + {() => {/*Ωignore_startΩ*/const ΩΩhello=hello;/*Ωignore_endΩ*/() => { let {hello} = __sveltets_instanceOf(Comp).$$slot_def['default'];/*Ωignore_startΩ*/(((Ωhello && hello1))) && ((ΩΩhello)) && /*Ωignore_endΩ*/<> {(hello) ? <> {hello} : <>} @@ -30,18 +30,18 @@ : <>} }}} {(hi && bye) ? <> - {() => {const Ωbye=bye;() => { let {foo:bye} = __sveltets_instanceOf(Comp).$$slot_def['default'];(((hello && hello1))) && ((hi && Ωbye)) && <> + {() => {/*Ωignore_startΩ*/const Ωbye=bye;/*Ωignore_endΩ*/() => { let {foo:bye} = __sveltets_instanceOf(Comp).$$slot_def['default'];/*Ωignore_startΩ*/(((hello && hello1))) && ((hi && Ωbye)) && /*Ωignore_endΩ*/<> {bye} }}} : (cool) ? <> - {() => {const Ωcool=cool,Ωhello=hello;() => { let {cool, hello} = __sveltets_instanceOf(Comp).$$slot_def['named'];(((Ωhello && hello1))) && (!(hi && bye) && (Ωcool)) && <>
+ {() => {/*Ωignore_startΩ*/const Ωcool=cool,Ωhello=hello;/*Ωignore_endΩ*/() => { let {cool, hello} = __sveltets_instanceOf(Comp).$$slot_def['named'];/*Ωignore_startΩ*/(((Ωhello && hello1))) && (!(hi && bye) && (Ωcool)) && /*Ωignore_endΩ*/<>
{hello}
}}} : <> - {() => {const Ωhello=hello;() => { let {foo:hello, hello1:other} = __sveltets_instanceOf(Comp).$$slot_def['named'];(((Ωhello && hello1))) && (!(hi && bye) && !(cool)) && <>
+ {() => {/*Ωignore_startΩ*/const Ωhello=hello;/*Ωignore_endΩ*/() => { let {foo:hello, hello1:other} = __sveltets_instanceOf(Comp).$$slot_def['named'];/*Ωignore_startΩ*/(((Ωhello && hello1))) && (!(hi && bye) && !(cool)) && /*Ωignore_endΩ*/<>
{hello}
}}} diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let/expected.jsx b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let/expected.jsx index 527498a80..768079481 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let/expected.jsx +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-slot-let/expected.jsx @@ -1,20 +1,20 @@ <>{(hello) ? <> - {() => { let {foo} = __sveltets_instanceOf(Comp).$$slot_def['default'];((hello)) && <> + {() => { let {foo} = __sveltets_instanceOf(Comp).$$slot_def['default'];/*Ωignore_startΩ*/((hello)) && /*Ωignore_endΩ*/<> {foo} }} {(hi && bye) ? <> - {() => { let {foo:bar} = __sveltets_instanceOf(Comp).$$slot_def['default'];(((hello))) && ((hi && bye)) && <> + {() => { let {foo:bar} = __sveltets_instanceOf(Comp).$$slot_def['default'];/*Ωignore_startΩ*/(((hello))) && ((hi && bye)) && /*Ωignore_endΩ*/<> {bar} }} : (cool) ? <> - {() => { let {foo, foo1} = __sveltets_instanceOf(Comp).$$slot_def['named'];(((hello))) && (!(hi && bye) && (cool)) && <>
+ {() => { let {foo, foo1} = __sveltets_instanceOf(Comp).$$slot_def['named'];/*Ωignore_startΩ*/(((hello))) && (!(hi && bye) && (cool)) && /*Ωignore_endΩ*/<>
{foo}
}} : <> - {() => { let {foo:bar} = __sveltets_instanceOf(Comp).$$slot_def['named'];(((hello))) && (!(hi && bye) && !(cool)) && <>
+ {() => { let {foo:bar} = __sveltets_instanceOf(Comp).$$slot_def['named'];/*Ωignore_startΩ*/(((hello))) && (!(hi && bye) && !(cool)) && /*Ωignore_endΩ*/<>
{bar}
}} diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/uses-svelte-components-let-forward/expected.tsx b/packages/svelte2tsx/test/svelte2tsx/samples/uses-svelte-components-let-forward/expected.tsx index 3262e56c0..5f6c5df29 100644 --- a/packages/svelte2tsx/test/svelte2tsx/samples/uses-svelte-components-let-forward/expected.tsx +++ b/packages/svelte2tsx/test/svelte2tsx/samples/uses-svelte-components-let-forward/expected.tsx @@ -1,7 +1,7 @@ /// <>;function render() { <>{(true) ? <> -{() => { let {prop} = __sveltets_instanceOf(__sveltets_componentType()).$$slot_def['default'];((true)) && <> +{() => { let {prop} = __sveltets_instanceOf(__sveltets_componentType()).$$slot_def['default'];/*Ωignore_startΩ*/((true)) && /*Ωignore_endΩ*/<> }} : <>} @@ -11,4 +11,4 @@ return { props: {}, slots: {'default': {prop:__sveltets_instanceOf(__sveltets_componentType()).$$slot_def['default'].prop}}, getters: {}, events: {} }} export default class Input__SvelteComponent_ extends createSvelte2TsxComponent(__sveltets_partial(__sveltets_with_any_event(render))) { -} +} \ No newline at end of file From 94e51ad7c257bc4d05f75da96e3b9aa8679c25e5 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 4 Mar 2021 13:04:35 +0100 Subject: [PATCH 14/17] codeactions/references/rename filter out generated add SnapshotFragmentMap util --- .../plugins/typescript/DocumentSnapshot.ts | 12 ++ .../plugins/typescript/TypeScriptPlugin.ts | 38 +++-- .../features/CodeActionsProvider.ts | 87 ++++++----- .../features/DiagnosticsProvider.ts | 8 +- .../features/FindReferencesProvider.ts | 21 +-- .../typescript/features/RenameProvider.ts | 36 ++--- .../features/UpdateImportsProvider.ts | 20 +-- .../src/plugins/typescript/features/utils.ts | 47 +++++- .../features/DiagnosticsProvider.test.ts | 144 +++++++++--------- .../features/FindReferencesProvider.test.ts | 49 ++++++ .../features/RenameProvider.test.ts | 57 +++++++ .../find-references-ignore-generated.svelte | 12 ++ .../rename/rename-ignore-generated.svelte | 12 ++ 13 files changed, 356 insertions(+), 187 deletions(-) create mode 100644 packages/language-server/test/plugins/typescript/testfiles/find-references-ignore-generated.svelte create mode 100644 packages/language-server/test/plugins/typescript/testfiles/rename/rename-ignore-generated.svelte diff --git a/packages/language-server/src/plugins/typescript/DocumentSnapshot.ts b/packages/language-server/src/plugins/typescript/DocumentSnapshot.ts index 0a01a7970..90ee9d4a5 100644 --- a/packages/language-server/src/plugins/typescript/DocumentSnapshot.ts +++ b/packages/language-server/src/plugins/typescript/DocumentSnapshot.ts @@ -57,6 +57,10 @@ export interface DocumentSnapshot extends ts.IScriptSnapshot { * in order to prevent memory leaks. */ destroyFragment(): void; + /** + * Convenience function for getText(0, getLength()) + */ + getFullText(): string; } /** @@ -221,6 +225,10 @@ export class SvelteDocumentSnapshot implements DocumentSnapshot { return this.text.length; } + getFullText() { + return this.text; + } + getChangeRange() { return undefined; } @@ -301,6 +309,10 @@ export class JSOrTSDocumentSnapshot return this.text.length; } + getFullText() { + return this.text; + } + getChangeRange() { return undefined; } diff --git a/packages/language-server/src/plugins/typescript/TypeScriptPlugin.ts b/packages/language-server/src/plugins/typescript/TypeScriptPlugin.ts index 5831e91e4..fb66043b4 100644 --- a/packages/language-server/src/plugins/typescript/TypeScriptPlugin.ts +++ b/packages/language-server/src/plugins/typescript/TypeScriptPlugin.ts @@ -28,7 +28,7 @@ import { getTextInRange } from '../../lib/documents'; import { LSConfigManager, LSTypescriptConfig } from '../../ls-config'; -import { pathToUrl } from '../../utils'; +import { isNotNullOrUndefined, pathToUrl } from '../../utils'; import { AppCompletionItem, AppCompletionList, @@ -49,7 +49,6 @@ import { SemanticTokensProvider, UpdateTsOrJsFile } from '../interfaces'; -import { SnapshotFragment } from './DocumentSnapshot'; import { CodeActionsProviderImpl } from './features/CodeActionsProvider'; import { CompletionEntryWithIdentifer, @@ -67,6 +66,7 @@ import { SelectionRangeProviderImpl } from './features/SelectionRangeProvider'; import { SignatureHelpProviderImpl } from './features/SignatureHelpProvider'; import { SnapshotManager } from './SnapshotManager'; import { SemanticTokensProviderImpl } from './features/SemanticTokensProvider'; +import { isNoTextSpanInGeneratedCode, SnapshotFragmentMap } from './features/utils'; export class TypeScriptPlugin implements @@ -263,35 +263,35 @@ export class TypeScriptPlugin } const { lang, tsDoc } = this.getLSAndTSDoc(document); - const fragment = await tsDoc.getFragment(); + const mainFragment = await tsDoc.getFragment(); const defs = lang.getDefinitionAndBoundSpan( tsDoc.filePath, - fragment.offsetAt(fragment.getGeneratedPosition(position)) + mainFragment.offsetAt(mainFragment.getGeneratedPosition(position)) ); if (!defs || !defs.definitions) { return []; } - const docs = new Map([[tsDoc.filePath, fragment]]); + const docs = new SnapshotFragmentMap(this.lsAndTsDocResolver); + docs.set(tsDoc.filePath, { fragment: mainFragment, snapshot: tsDoc }); - return await Promise.all( + const result = await Promise.all( defs.definitions.map(async (def) => { - let defDoc = docs.get(def.fileName); - if (!defDoc) { - defDoc = await this.getSnapshot(def.fileName).getFragment(); - docs.set(def.fileName, defDoc); + const { fragment, snapshot } = await docs.retrieve(def.fileName); + + if (isNoTextSpanInGeneratedCode(snapshot.getFullText(), def.textSpan)) { + return LocationLink.create( + pathToUrl(def.fileName), + convertToLocationRange(fragment, def.textSpan), + convertToLocationRange(fragment, def.textSpan), + convertToLocationRange(mainFragment, defs.textSpan) + ); } - - return LocationLink.create( - pathToUrl(def.fileName), - convertToLocationRange(defDoc, def.textSpan), - convertToLocationRange(defDoc, def.textSpan), - convertToLocationRange(fragment, defs.textSpan) - ); }) ); + return result.filter(isNotNullOrUndefined); } async prepareRename(document: Document, position: Position): Promise { @@ -436,10 +436,6 @@ export class TypeScriptPlugin return this.lsAndTsDocResolver.getLSAndTSDoc(document); } - private getSnapshot(filePath: string, document?: Document) { - return this.lsAndTsDocResolver.getSnapshot(filePath, document); - } - /** * * @internal diff --git a/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts b/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts index dac8c9d6b..cdae021be 100644 --- a/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts +++ b/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts @@ -9,7 +9,7 @@ import { WorkspaceEdit } from 'vscode-languageserver'; import { Document, mapRangeToOriginal, isRangeInTag, isInTag } from '../../../lib/documents'; -import { pathToUrl, flatten } from '../../../utils'; +import { pathToUrl, flatten, isNotNullOrUndefined } from '../../../utils'; import { CodeActionsProvider } from '../../interfaces'; import { SnapshotFragment, SvelteSnapshotFragment } from '../DocumentSnapshot'; import { LSAndTSDocResolver } from '../LSAndTSDocResolver'; @@ -17,6 +17,7 @@ import { convertRange } from '../utils'; import ts from 'typescript'; import { CompletionsProviderImpl } from './CompletionProvider'; +import { isNoTextSpanInGeneratedCode, SnapshotFragmentMap } from './utils'; interface RefactorArgs { type: 'refactor'; @@ -134,45 +135,56 @@ export class CodeActionsProviderImpl implements CodeActionsProvider { userPreferences ); - const docs = new Map([[tsDoc.filePath, fragment]]); + const docs = new SnapshotFragmentMap(this.lsAndTsDocResolver); + docs.set(tsDoc.filePath, { fragment, snapshot: tsDoc }); + return await Promise.all( codeFixes.map(async (fix) => { const documentChanges = await Promise.all( fix.changes.map(async (change) => { - const doc = - docs.get(change.fileName) ?? - (await this.getAndCacheCodeActionDoc(change, docs)); + const { snapshot, fragment } = await docs.retrieve(change.fileName); return TextDocumentEdit.create( VersionedTextDocumentIdentifier.create(pathToUrl(change.fileName), 0), - change.textChanges.map((edit) => { - if ( - fix.fixName === 'import' && - doc instanceof SvelteSnapshotFragment - ) { - return this.completionProvider.codeActionChangeToTextEdit( - document, - doc, - edit, - true, - isInTag(range.start, document.scriptInfo) || - isInTag(range.start, document.moduleScriptInfo) + change.textChanges + .map((edit) => { + if ( + fix.fixName === 'import' && + fragment instanceof SvelteSnapshotFragment + ) { + return this.completionProvider.codeActionChangeToTextEdit( + document, + fragment, + edit, + true, + isInTag(range.start, document.scriptInfo) || + isInTag(range.start, document.moduleScriptInfo) + ); + } + + if ( + !isNoTextSpanInGeneratedCode( + snapshot.getFullText(), + edit.span + ) + ) { + return undefined; + } + + let originalRange = mapRangeToOriginal( + fragment, + convertRange(fragment, edit.span) ); - } - - let originalRange = mapRangeToOriginal( - doc, - convertRange(doc, edit.span) - ); - if (fix.fixName === 'unusedIdentifier') { - originalRange = this.checkRemoveImportCodeActionRange( - edit, - doc, - originalRange - ); - } - - return TextEdit.replace(originalRange, edit.newText); - }) + if (fix.fixName === 'unusedIdentifier') { + originalRange = this.checkRemoveImportCodeActionRange( + edit, + fragment, + originalRange + ); + } + + return TextEdit.replace(originalRange, edit.newText); + }) + .filter(isNotNullOrUndefined) ); }) ); @@ -187,15 +199,6 @@ export class CodeActionsProviderImpl implements CodeActionsProvider { ); } - private async getAndCacheCodeActionDoc( - change: ts.FileTextChanges, - cache: Map - ) { - const doc = await this.getSnapshot(change.fileName).getFragment(); - cache.set(change.fileName, doc); - return doc; - } - private async getApplicableRefactors(document: Document, range: Range): Promise { if ( !isRangeInTag(range, document.scriptInfo) && diff --git a/packages/language-server/src/plugins/typescript/features/DiagnosticsProvider.ts b/packages/language-server/src/plugins/typescript/features/DiagnosticsProvider.ts index c8140bf63..20c6cf274 100644 --- a/packages/language-server/src/plugins/typescript/features/DiagnosticsProvider.ts +++ b/packages/language-server/src/plugins/typescript/features/DiagnosticsProvider.ts @@ -1,12 +1,6 @@ import ts from 'typescript'; import { Diagnostic, DiagnosticSeverity, DiagnosticTag } from 'vscode-languageserver'; -import { - Document, - mapObjWithRangeToOriginal, - getTextInRange, - getLineAtPosition, - offsetAt -} from '../../../lib/documents'; +import { Document, mapObjWithRangeToOriginal, getTextInRange } from '../../../lib/documents'; import { DiagnosticsProvider } from '../../interfaces'; import { LSAndTSDocResolver } from '../LSAndTSDocResolver'; import { convertRange, mapSeverity } from '../utils'; diff --git a/packages/language-server/src/plugins/typescript/features/FindReferencesProvider.ts b/packages/language-server/src/plugins/typescript/features/FindReferencesProvider.ts index f90b24e54..21a70c6d7 100644 --- a/packages/language-server/src/plugins/typescript/features/FindReferencesProvider.ts +++ b/packages/language-server/src/plugins/typescript/features/FindReferencesProvider.ts @@ -1,10 +1,11 @@ +import ts from 'typescript'; import { Location, Position, ReferenceContext } from 'vscode-languageserver'; import { Document } from '../../../lib/documents'; import { pathToUrl } from '../../../utils'; import { FindReferencesProvider } from '../../interfaces'; -import { SnapshotFragment } from '../DocumentSnapshot'; import { LSAndTSDocResolver } from '../LSAndTSDocResolver'; import { convertToLocationRange } from '../utils'; +import { isNoTextSpanInGeneratedCode, SnapshotFragmentMap } from './utils'; export class FindReferencesProviderImpl implements FindReferencesProvider { constructor(private readonly lsAndTsDocResolver: LSAndTSDocResolver) {} @@ -25,17 +26,15 @@ export class FindReferencesProviderImpl implements FindReferencesProvider { return null; } - const docs = new Map([[tsDoc.filePath, fragment]]); + const docs = new SnapshotFragmentMap(this.lsAndTsDocResolver); + docs.set(tsDoc.filePath, { fragment, snapshot: tsDoc }); return await Promise.all( references .filter((ref) => context.includeDeclaration || !ref.isDefinition) + .filter(notInGeneratedCode(tsDoc.getFullText())) .map(async (ref) => { - let defDoc = docs.get(ref.fileName); - if (!defDoc) { - defDoc = await this.getSnapshot(ref.fileName).getFragment(); - docs.set(ref.fileName, defDoc); - } + const defDoc = await docs.retrieveFragment(ref.fileName); return Location.create( pathToUrl(ref.fileName), @@ -48,8 +47,10 @@ export class FindReferencesProviderImpl implements FindReferencesProvider { private getLSAndTSDoc(document: Document) { return this.lsAndTsDocResolver.getLSAndTSDoc(document); } +} - private getSnapshot(filePath: string, document?: Document) { - return this.lsAndTsDocResolver.getSnapshot(filePath, document); - } +function notInGeneratedCode(text: string) { + return (ref: ts.ReferenceEntry) => { + return isNoTextSpanInGeneratedCode(text, ref.textSpan); + }; } diff --git a/packages/language-server/src/plugins/typescript/features/RenameProvider.ts b/packages/language-server/src/plugins/typescript/features/RenameProvider.ts index 3216b4e3a..0d9175234 100644 --- a/packages/language-server/src/plugins/typescript/features/RenameProvider.ts +++ b/packages/language-server/src/plugins/typescript/features/RenameProvider.ts @@ -6,7 +6,7 @@ import { offsetAt, getLineAtPosition } from '../../../lib/documents'; -import { pathToUrl } from '../../../utils'; +import { isNotNullOrUndefined, pathToUrl } from '../../../utils'; import { RenameProvider } from '../../interfaces'; import { SnapshotFragment, @@ -17,6 +17,7 @@ import { convertRange } from '../utils'; import { LSAndTSDocResolver } from '../LSAndTSDocResolver'; import ts from 'typescript'; import { uniqWith, isEqual } from 'lodash'; +import { isNoTextSpanInGeneratedCode, SnapshotFragmentMap } from './utils'; export class RenameProviderImpl implements RenameProvider { constructor(private readonly lsAndTsDocResolver: LSAndTSDocResolver) {} @@ -61,7 +62,8 @@ export class RenameProviderImpl implements RenameProvider { return null; } - const docs = new Map([[tsDoc.filePath, fragment]]); + const docs = new SnapshotFragmentMap(this.lsAndTsDocResolver); + docs.set(tsDoc.filePath, { fragment, snapshot: tsDoc }); let convertedRenameLocations: Array< ts.RenameLocation & { range: Range; @@ -158,7 +160,7 @@ export class RenameProviderImpl implements RenameProvider { fragment: SvelteSnapshotFragment, position: Position, convertedRenameLocations: Array, - fragments: Map, + fragments: SnapshotFragmentMap, lang: ts.LanguageService ) { // First find out if it's really the "rename prop inside component with that prop" case @@ -214,7 +216,7 @@ export class RenameProviderImpl implements RenameProvider { */ private async getAdditionalLocationsForRenameOfPropInsideOtherComponent( convertedRenameLocations: Array, - fragments: Map, + fragments: SnapshotFragmentMap, lang: ts.LanguageService ) { // Check if it's a prop rename @@ -226,7 +228,7 @@ export class RenameProviderImpl implements RenameProvider { return []; } // Find generated `export let` - const doc = fragments.get(updatePropLocation.fileName); + const doc = fragments.getFragment(updatePropLocation.fileName); const match = this.matchGeneratedExportLet(doc, updatePropLocation); if (!match) { return []; @@ -256,7 +258,7 @@ export class RenameProviderImpl implements RenameProvider { private findLocationWhichWantsToUpdatePropName( convertedRenameLocations: Array, - fragments: Map + fragments: SnapshotFragmentMap ) { return convertedRenameLocations.find((loc) => { // Props are not in mapped range @@ -264,7 +266,7 @@ export class RenameProviderImpl implements RenameProvider { return; } - const fragment = fragments.get(loc.fileName); + const fragment = fragments.getFragment(loc.fileName); // Props are in svelte snapshots only if (!(fragment instanceof SvelteSnapshotFragment)) { return false; @@ -295,23 +297,21 @@ export class RenameProviderImpl implements RenameProvider { */ private async mapAndFilterRenameLocations( renameLocations: readonly ts.RenameLocation[], - fragments: Map + fragments: SnapshotFragmentMap ): Promise> { const mappedLocations = await Promise.all( renameLocations.map(async (loc) => { - let doc = fragments.get(loc.fileName); - if (!doc) { - doc = await this.getSnapshot(loc.fileName).getFragment(); - fragments.set(loc.fileName, doc); - } + const { fragment, snapshot } = await fragments.retrieve(loc.fileName); - return { - ...loc, - range: this.mapRangeToOriginal(doc, loc.textSpan) - }; + if (isNoTextSpanInGeneratedCode(snapshot.getFullText(), loc.textSpan)) { + return { + ...loc, + range: this.mapRangeToOriginal(fragment, loc.textSpan) + }; + } }) ); - return this.filterWrongRenameLocations(mappedLocations); + return this.filterWrongRenameLocations(mappedLocations.filter(isNotNullOrUndefined)); } private filterWrongRenameLocations( diff --git a/packages/language-server/src/plugins/typescript/features/UpdateImportsProvider.ts b/packages/language-server/src/plugins/typescript/features/UpdateImportsProvider.ts index e36f8acdf..d473f9d2d 100644 --- a/packages/language-server/src/plugins/typescript/features/UpdateImportsProvider.ts +++ b/packages/language-server/src/plugins/typescript/features/UpdateImportsProvider.ts @@ -4,12 +4,12 @@ import { VersionedTextDocumentIdentifier, WorkspaceEdit } from 'vscode-languageserver'; -import { Document, mapRangeToOriginal } from '../../../lib/documents'; +import { mapRangeToOriginal } from '../../../lib/documents'; import { urlToPath } from '../../../utils'; import { FileRename, UpdateImportsProvider } from '../../interfaces'; -import { SnapshotFragment } from '../DocumentSnapshot'; import { LSAndTSDocResolver } from '../LSAndTSDocResolver'; import { convertRange } from '../utils'; +import { SnapshotFragmentMap } from './utils'; export class UpdateImportsProviderImpl implements UpdateImportsProvider { constructor(private readonly lsAndTsDocResolver: LSAndTSDocResolver) {} @@ -37,21 +37,17 @@ export class UpdateImportsProviderImpl implements UpdateImportsProvider { return change; }); - const docs = new Map(); + const docs = new SnapshotFragmentMap(this.lsAndTsDocResolver); const documentChanges = await Promise.all( updateImportsChanges.map(async (change) => { - let fragment = docs.get(change.fileName); - if (!fragment) { - fragment = await this.getSnapshot(change.fileName).getFragment(); - docs.set(change.fileName, fragment); - } + const fragment = await docs.retrieveFragment(change.fileName); return TextDocumentEdit.create( VersionedTextDocumentIdentifier.create(fragment.getURL(), 0), change.textChanges.map((edit) => { const range = mapRangeToOriginal( - fragment!, - convertRange(fragment!, edit.span) + fragment, + convertRange(fragment, edit.span) ); return TextEdit.replace(range, edit.newText); }) @@ -65,8 +61,4 @@ export class UpdateImportsProviderImpl implements UpdateImportsProvider { private getLSForPath(path: string) { return this.lsAndTsDocResolver.getLSForPath(path); } - - private getSnapshot(filePath: string, document?: Document) { - return this.lsAndTsDocResolver.getSnapshot(filePath, document); - } } diff --git a/packages/language-server/src/plugins/typescript/features/utils.ts b/packages/language-server/src/plugins/typescript/features/utils.ts index d06895fc4..fc014dd4a 100644 --- a/packages/language-server/src/plugins/typescript/features/utils.ts +++ b/packages/language-server/src/plugins/typescript/features/utils.ts @@ -1,7 +1,12 @@ import ts from 'typescript'; import { Position } from 'vscode-languageserver'; import { Document, getNodeIfIsInComponentStartTag, isInTag } from '../../../lib/documents'; -import { SvelteDocumentSnapshot, SvelteSnapshotFragment } from '../DocumentSnapshot'; +import { + DocumentSnapshot, + SnapshotFragment, + SvelteDocumentSnapshot, + SvelteSnapshotFragment +} from '../DocumentSnapshot'; import { LSAndTSDocResolver } from '../LSAndTSDocResolver'; /** @@ -61,3 +66,43 @@ export function isInGeneratedCode(text: string, start: number, end: number) { text.substring(end, lineEnd).includes('/*Ωignore_endΩ*/') ); } + +/** + * Checks that this isn't a text span that should be completely ignored + * because it's purely generated. + */ +export function isNoTextSpanInGeneratedCode(text: string, span: ts.TextSpan) { + return !isInGeneratedCode(text, span.start, span.start + span.length); +} + +export class SnapshotFragmentMap { + private map = new Map(); + constructor(private resolver: LSAndTSDocResolver) {} + + set(fileName: string, content: { fragment: SnapshotFragment; snapshot: DocumentSnapshot }) { + this.map.set(fileName, content); + } + + get(fileName: string) { + return this.map.get(fileName); + } + + getFragment(fileName: string) { + return this.map.get(fileName)?.fragment; + } + + async retrieve(fileName: string) { + let snapshotFragment = this.get(fileName); + if (!snapshotFragment) { + const snapshot = this.resolver.getSnapshot(fileName); + const fragment = await snapshot.getFragment(); + snapshotFragment = { fragment, snapshot }; + this.set(fileName, snapshotFragment); + } + return snapshotFragment; + } + + async retrieveFragment(fileName: string) { + return (await this.retrieve(fileName)).fragment; + } +} diff --git a/packages/language-server/test/plugins/typescript/features/DiagnosticsProvider.test.ts b/packages/language-server/test/plugins/typescript/features/DiagnosticsProvider.test.ts index 253e06739..948d92e78 100644 --- a/packages/language-server/test/plugins/typescript/features/DiagnosticsProvider.test.ts +++ b/packages/language-server/test/plugins/typescript/features/DiagnosticsProvider.test.ts @@ -711,79 +711,75 @@ describe('DiagnosticsProvider', () => { const { plugin, document } = setup('diagnostics-ignore-generated.svelte'); const diagnostics = await plugin.getDiagnostics(document); - assert.deepStrictEqual( - diagnostics, - - [ - { - code: 2304, - message: "Cannot find name 'a'.", - range: { - end: { - character: 13, - line: 3 - }, - start: { - character: 12, - line: 3 - } - }, - severity: 1, - source: 'ts', - tags: [] - }, - { - code: 2304, - message: "Cannot find name 'a'.", - range: { - end: { - character: 6, - line: 4 - }, - start: { - character: 5, - line: 4 - } - }, - severity: 1, - source: 'ts', - tags: [] - }, - { - code: 2304, - message: "Cannot find name 'b'.", - range: { - end: { - character: 10, - line: 8 - }, - start: { - character: 9, - line: 8 - } - }, - severity: 1, - source: 'ts', - tags: [] - }, - { - code: 2304, - message: "Cannot find name 'b'.", - range: { - end: { - character: 10, - line: 9 - }, - start: { - character: 9, - line: 9 - } - }, - severity: 1, - source: 'ts', - tags: [] - } - ] - ); + assert.deepStrictEqual(diagnostics, [ + { + code: 2304, + message: "Cannot find name 'a'.", + range: { + end: { + character: 13, + line: 3 + }, + start: { + character: 12, + line: 3 + } + }, + severity: 1, + source: 'ts', + tags: [] + }, + { + code: 2304, + message: "Cannot find name 'a'.", + range: { + end: { + character: 6, + line: 4 + }, + start: { + character: 5, + line: 4 + } + }, + severity: 1, + source: 'ts', + tags: [] + }, + { + code: 2304, + message: "Cannot find name 'b'.", + range: { + end: { + character: 10, + line: 8 + }, + start: { + character: 9, + line: 8 + } + }, + severity: 1, + source: 'ts', + tags: [] + }, + { + code: 2304, + message: "Cannot find name 'b'.", + range: { + end: { + character: 10, + line: 9 + }, + start: { + character: 9, + line: 9 + } + }, + severity: 1, + source: 'ts', + tags: [] + } + ]); }); }); diff --git a/packages/language-server/test/plugins/typescript/features/FindReferencesProvider.test.ts b/packages/language-server/test/plugins/typescript/features/FindReferencesProvider.test.ts index 44cb68133..a4170c6a7 100644 --- a/packages/language-server/test/plugins/typescript/features/FindReferencesProvider.test.ts +++ b/packages/language-server/test/plugins/typescript/features/FindReferencesProvider.test.ts @@ -156,4 +156,53 @@ describe('FindReferencesProvider', () => { } ]); }); + + it('ignores references inside generated code', async () => { + const { provider, document } = setup('find-references-ignore-generated.svelte'); + + const results = await provider.findReferences(document, Position.create(1, 8), { + includeDeclaration: true + }); + assert.deepStrictEqual(results, [ + { + range: { + end: { + character: 9, + line: 1 + }, + start: { + character: 8, + line: 1 + } + }, + uri: getUri('find-references-ignore-generated.svelte') + }, + { + range: { + end: { + character: 6, + line: 5 + }, + start: { + character: 5, + line: 5 + } + }, + uri: getUri('find-references-ignore-generated.svelte') + }, + { + range: { + end: { + character: 21, + line: 7 + }, + start: { + character: 20, + line: 7 + } + }, + uri: getUri('find-references-ignore-generated.svelte') + } + ]); + }); }); diff --git a/packages/language-server/test/plugins/typescript/features/RenameProvider.test.ts b/packages/language-server/test/plugins/typescript/features/RenameProvider.test.ts index 3b811e268..197abb1d8 100644 --- a/packages/language-server/test/plugins/typescript/features/RenameProvider.test.ts +++ b/packages/language-server/test/plugins/typescript/features/RenameProvider.test.ts @@ -35,6 +35,7 @@ describe('RenameProvider', () => { const renameDoc4 = await openDoc('rename4.svelte'); const renameDoc5 = await openDoc('rename5.svelte'); const renameDoc6 = await openDoc('rename6.svelte'); + const renameDocIgnoreGenerated = await openDoc('rename-ignore-generated.svelte'); return { provider, renameDoc1, @@ -43,6 +44,7 @@ describe('RenameProvider', () => { renameDoc4, renameDoc5, renameDoc6, + renameDocIgnoreGenerated, docManager }; @@ -469,4 +471,59 @@ describe('RenameProvider', () => { } }); }); + + it('should rename and ignore generated', async () => { + const { provider, renameDocIgnoreGenerated } = await setup(); + const result = await provider.rename( + renameDocIgnoreGenerated, + Position.create(1, 8), + 'newName' + ); + + assert.deepStrictEqual(result, { + changes: { + [getUri('rename-ignore-generated.svelte')]: [ + { + newText: 'newName', + range: { + end: { + character: 9, + line: 1 + }, + start: { + character: 8, + line: 1 + } + } + }, + { + newText: 'newName', + range: { + end: { + character: 6, + line: 5 + }, + start: { + character: 5, + line: 5 + } + } + }, + { + newText: 'newName', + range: { + end: { + character: 21, + line: 7 + }, + start: { + character: 20, + line: 7 + } + } + } + ] + } + }); + }); }); diff --git a/packages/language-server/test/plugins/typescript/testfiles/find-references-ignore-generated.svelte b/packages/language-server/test/plugins/typescript/testfiles/find-references-ignore-generated.svelte new file mode 100644 index 000000000..693e7a15b --- /dev/null +++ b/packages/language-server/test/plugins/typescript/testfiles/find-references-ignore-generated.svelte @@ -0,0 +1,12 @@ + + +{#if a} + {#await promise} + {#if typeof a === 'string'} + {promise} + {/if} + {/await} +{/if} diff --git a/packages/language-server/test/plugins/typescript/testfiles/rename/rename-ignore-generated.svelte b/packages/language-server/test/plugins/typescript/testfiles/rename/rename-ignore-generated.svelte new file mode 100644 index 000000000..693e7a15b --- /dev/null +++ b/packages/language-server/test/plugins/typescript/testfiles/rename/rename-ignore-generated.svelte @@ -0,0 +1,12 @@ + + +{#if a} + {#await promise} + {#if typeof a === 'string'} + {promise} + {/if} + {/await} +{/if} From 2a43f4e0f494a8390c4adde546af5ba80c889fc6 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 8 Mar 2021 15:50:35 +0100 Subject: [PATCH 15/17] more docs --- .../src/htmlxtojsx/nodes/if-scope.ts | 85 +++++++++++++++++-- 1 file changed, 78 insertions(+), 7 deletions(-) diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts index 778bf09ab..c29da4b3e 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts @@ -10,28 +10,103 @@ enum IfType { Else } +/** + * Contains the raw text of the condition and a map of identifiers + * used within that condition. + */ interface ConditionInfo { identifiers: Map>; text: string; } +/** + * See `Condition` for an explanation of the structure. + */ interface IfCondition { type: IfType.If; condition: ConditionInfo; } +/** + * See `Condition` for an explanation of the structure. + */ interface ElseIfCondition { type: IfType.ElseIf; condition: ConditionInfo; parent: IfCondition | ElseIfCondition; } +/** + * See `Condition` for an explanation of the structure. + */ interface ElseCondition { type: IfType.Else; parent: IfCondition | ElseIfCondition; } + +/** + * A condition is a nested structure which starts with IfCondition + * and ends with ElseIfCondition or ElseCondition. + * ElseIfCondition/ElseCondition have parents which mark the previous conditions. + * This means that the resulting structure is reversed (starts with the last condition). + * Example: + * + * ``` + * if (foo) { + * } else if (bar) { + * } else {} + * ``` + * + * is translated to + * + * ``` + * { + * type: 2, // Else + * parent: { + * type: 1, // ElseIf + * condition: { + * identifiers: [[bar, [{start: 0, end: 2}]]], + * text: 'bar' + * }, + * parent: { + * type: 0, // If + * condition: { + * identifiers: [[foo, [{start: 0, end: 2}]]], + * text: 'foo' + * } + * } + * ``` + */ type Condition = IfCondition | ElseIfCondition | ElseCondition; +/** + * Creates a new condition whos parent is the current condition + * and the leaf is the passed in condition info. + * See `Condition` for an explanation of the structure. + */ +function addElseIfCondition( + existingCondition: IfCondition | ElseIfCondition, + newCondition: ConditionInfo +): ElseIfCondition { + return { + parent: existingCondition, + condition: newCondition, + type: IfType.ElseIf + }; +} + +/** + * Creates a new condition whos parent is the current condition + * and the leaf is the else condition, where children can follow. + * See `Condition` for an explanation of the structure. + */ +function addElseCondition(existingCondition: IfCondition | ElseIfCondition): ElseCondition { + return { + parent: existingCondition, + type: IfType.Else + }; +} + const REPLACEMENT_PREFIX = '\u03A9'; /** @@ -210,18 +285,14 @@ export class IfScope { */ addElseIf(expression: Node, str: MagicString): void { const condition = this.getConditionInfo(str, expression); - this.current = { - condition, - parent: this.current as IfCondition | ElseIfCondition, - type: IfType.ElseIf - }; + this.current = addElseIfCondition(this.current as IfCondition | ElseIfCondition, condition); } /** * Adds a `else` branch to the scope and enhances the condition accordingly. */ addElse(): void { - this.current = { parent: this.current as IfCondition | ElseIfCondition, type: IfType.Else }; + this.current = addElseCondition(this.current as IfCondition | ElseIfCondition); } getChild(): IfScope { @@ -273,7 +344,7 @@ export class IfScope { return this.parent.referencesIdentifier(name); } - private getConditionInfo(str: MagicString, expression: Node) { + private getConditionInfo(str: MagicString, expression: Node): ConditionInfo { const identifiers = getIdentifiersInIfExpression(expression); const text = str.original.substring(expression.start, expression.end); return { identifiers, text }; From 94210c813bc9866ca3fb8d926948278768eccdca Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Tue, 9 Mar 2021 10:35:10 +0100 Subject: [PATCH 16/17] fix non-shadowed catch/then cases --- packages/svelte2tsx/src/htmlxtojsx/index.ts | 14 ++- .../svelte2tsx/src/htmlxtojsx/nodes/await.ts | 101 +++++++++++------- .../src/htmlxtojsx/nodes/if-scope.ts | 22 ++-- .../src/htmlxtojsx/nodes/template-scope.ts | 32 ++++++ .../expected.jsx | 18 +++- .../input.svelte | 10 ++ .../if-nested-await-block/expected.jsx | 2 +- 7 files changed, 143 insertions(+), 56 deletions(-) diff --git a/packages/svelte2tsx/src/htmlxtojsx/index.ts b/packages/svelte2tsx/src/htmlxtojsx/index.ts index 94d0f095c..81eb06f62 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/index.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/index.ts @@ -6,7 +6,7 @@ import { getSlotName } from '../utils/svelteAst'; import { handleActionDirective } from './nodes/action-directive'; import { handleAnimateDirective } from './nodes/animation-directive'; import { handleAttribute } from './nodes/attribute'; -import { handleAwait } from './nodes/await'; +import { handleAwait, handleAwaitCatch, handleAwaitPending, handleAwaitThen } from './nodes/await'; import { handleBinding } from './nodes/binding'; import { handleClassDirective } from './nodes/class-directive'; import { handleComment } from './nodes/comment'; @@ -77,6 +77,18 @@ export function convertHtmlxToJsx( templateScopeManager.awaitEnter(node); handleAwait(htmlx, str, node, ifScope); break; + case 'PendingBlock': + templateScopeManager.awaitPendingEnter(node, parent); + handleAwaitPending(parent, htmlx, str, ifScope); + break; + case 'ThenBlock': + templateScopeManager.awaitThenEnter(node, parent); + handleAwaitThen(parent, htmlx, str, ifScope); + break; + case 'CatchBlock': + templateScopeManager.awaitCatchEnter(node, parent); + handleAwaitCatch(parent, htmlx, str, ifScope); + break; case 'KeyBlock': handleKey(htmlx, str, node); break; diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/await.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/await.ts index b7ee1d2b7..67bcbd7c1 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/await.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/await.ts @@ -20,46 +20,70 @@ export function handleAwait( `{() => {${constRedeclares}let _$$p = (` ); + // {/await} -> + // <>})} + const awaitEndStart = htmlx.lastIndexOf('{', awaitBlock.end - 1); + str.overwrite(awaitEndStart, awaitBlock.end, '})}}'); +} + +export function handleAwaitPending( + awaitBlock: Node, + htmlx: string, + str: MagicString, + ifScope: IfScope +): void { + if (awaitBlock.pending.skip) { + return; + } + + // {await aPromise} ... -> aPromise); (possibleIfCondition &&)<> ... + const pendingStart = htmlx.indexOf('}', awaitBlock.expression.end); + const pendingEnd = !awaitBlock.then.skip + ? awaitBlock.then.start + : !awaitBlock.catch.skip + ? awaitBlock.catch.start + : htmlx.lastIndexOf('{', awaitBlock.end); + str.overwrite(awaitBlock.expression.end, pendingStart + 1, ');'); + str.appendRight(pendingStart + 1, ` ${ifScope.addPossibleIfCondition()}<>`); + str.appendLeft(pendingEnd, '; '); + + if (!awaitBlock.then.skip) { + return; + } + // no need to prepend ifcondition here as we know the then block is empty + str.appendLeft(pendingEnd, '__sveltets_awaitThen(_$$p, () => {<>'); +} + +export function handleAwaitThen( + awaitBlock: Node, + htmlx: string, + str: MagicString, + ifScope: IfScope +): void { + if (awaitBlock.then.skip) { + return; + } + // then value } | {:then value} | {await ..} .. {/await} -> // __sveltets_awaitThen(_$$p, (value) => {(possibleIfCondition && )<> let thenStart: number; let thenEnd: number; - if (!awaitBlock.then.skip) { - // then value } | {:then value} - if (!awaitBlock.pending.skip) { - // {await ...} ... {:then ...} - // thenBlock includes the {:then} - thenStart = awaitBlock.then.start; - if (awaitBlock.value) { - thenEnd = htmlx.indexOf('}', awaitBlock.value.end) + 1; - } else { - thenEnd = htmlx.indexOf('}', awaitBlock.then.start) + 1; - } - str.prependLeft(thenStart, '; '); - // add the start tag too - const awaitEnd = htmlx.indexOf('}', awaitBlock.expression.end); - - // somePromise} -> somePromise); - str.overwrite(awaitBlock.expression.end, awaitEnd + 1, ');'); - str.appendRight(awaitEnd + 1, ` ${ifScope.addPossibleIfCondition(true)}<>`); + // then value } | {:then value} + if (!awaitBlock.pending.skip) { + // {await ...} ... {:then ...} + // thenBlock includes the {:then} + thenStart = awaitBlock.then.start; + if (awaitBlock.value) { + thenEnd = htmlx.indexOf('}', awaitBlock.value.end) + 1; } else { - // {await ... then ...} - thenStart = htmlx.indexOf('then', awaitBlock.expression.end); - thenEnd = htmlx.lastIndexOf('}', awaitBlock.then.start) + 1; - // somePromise then -> somePromise); then - str.overwrite(awaitBlock.expression.end, thenStart, '); '); + thenEnd = htmlx.indexOf('}', awaitBlock.then.start) + 1; } } else { - // {await ..} ... ({:catch ..}) {/await} -> no then block, no value, but always a pending block - thenEnd = awaitBlock.catch.skip - ? htmlx.lastIndexOf('{', awaitBlock.end) - : awaitBlock.catch.start; - thenStart = Math.min(awaitBlock.pending.end + 1, thenEnd); - - const awaitEnd = htmlx.indexOf('}', awaitBlock.expression.end); - str.overwrite(awaitBlock.expression.end, awaitEnd + 1, ');'); - str.appendRight(awaitEnd + 1, ` ${ifScope.addPossibleIfCondition(true)}<>`); - str.appendLeft(thenEnd, '; '); + // {await ... then ...} + thenStart = htmlx.indexOf('then', awaitBlock.expression.end); + thenEnd = htmlx.lastIndexOf('}', awaitBlock.then.start) + 1; + // somePromise then -> somePromise); then + str.overwrite(awaitBlock.expression.end, thenStart, '); '); } if (awaitBlock.value) { @@ -73,7 +97,14 @@ export function handleAwait( str.overwrite(thenStart, thenEnd, awaitThenFn); } } +} +export function handleAwaitCatch( + awaitBlock: Node, + htmlx: string, + str: MagicString, + ifScope: IfScope +): void { //{:catch error} -> //}, (error) => {<> if (!awaitBlock.catch.skip) { @@ -87,8 +118,4 @@ export function handleAwait( str.overwrite(catchStart, errorStart, '}, ('); str.overwrite(errorEnd, catchEnd, `) => {${ifScope.addPossibleIfCondition()}<>`); } - // {/await} -> - // <>})} - const awaitEndStart = htmlx.lastIndexOf('{', awaitBlock.end - 1); - str.overwrite(awaitEndStart, awaitBlock.end, '})}}'); } diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts index c29da4b3e..9fdd39aa5 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/if-scope.ts @@ -248,15 +248,15 @@ export class IfScope { * Returns the full currently known condition, prepended with the conditions * of its parents. Identifiers in the condition get replaced if they were redeclared. */ - getFullCondition(skipImmediateChildScope = false): string { + getFullCondition(): string { if (!this.current) { return ''; } - const parentCondition = this.parent?.getFullCondition(false); + const parentCondition = this.parent?.getFullCondition(); const condition = `(${getFullCondition( this.current, - this.getNamesThatNeedReplacement(skipImmediateChildScope), + this.getNamesThatNeedReplacement(), this.replacementPrefix )})`; return parentCondition ? `(${parentCondition}) && ${condition}` : condition; @@ -266,8 +266,8 @@ export class IfScope { * Convenience method which invokes `getFullCondition` and adds a `&&` at the end * for easy chaining. */ - addPossibleIfCondition(skipImmediateChildScope = false): string { - const condition = this.getFullCondition(skipImmediateChildScope); + addPossibleIfCondition(): string { + const condition = this.getFullCondition(); return condition ? surroundWithIgnoreComments(`${condition} && `) : ''; } @@ -369,10 +369,10 @@ export class IfScope { /** * Return all identifiers that were redeclared and therefore need replacement. */ - private getNamesThatNeedReplacement(skipImmediateChildScope: boolean) { + private getNamesThatNeedReplacement() { const referencedIdentifiers = this.collectReferencedIdentifiers(); return [...referencedIdentifiers].filter((identifier) => - this.someChildScopeHasRedeclaredVariable(identifier, skipImmediateChildScope) + this.someChildScopeHasRedeclaredVariable(identifier) ); } @@ -380,13 +380,9 @@ export class IfScope { * Returns true if given identifier name is redeclared in a child template scope * and is therefore shadowed within that scope. */ - private someChildScopeHasRedeclaredVariable(name: string, skipImmediateChildScope: boolean) { + private someChildScopeHasRedeclaredVariable(name: string) { let scope = this.scope.value; - while ( - scope && - (!skipImmediateChildScope || scope.parent !== this.ownScope) && - scope !== this.ownScope - ) { + while (scope && scope !== this.ownScope) { if (scope.inits.has(name)) { return true; } diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/template-scope.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/template-scope.ts index 478b6c871..c76c8e792 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/template-scope.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/template-scope.ts @@ -57,6 +57,38 @@ export class TemplateScopeManager { } } + awaitPendingEnter(node: Node, parent: Node) { + if (node.skip || parent.type !== 'AwaitBlock') { + return; + } + // Reset inits, as pending can have no inits + this.value.inits.clear(); + } + + awaitThenEnter(node: Node, parent: Node) { + if (node.skip || parent.type !== 'AwaitBlock') { + return; + } + // Reset inits, this time only taking the then + // scope into account. + this.value.inits.clear(); + if (parent.value) { + this.handleScope(parent.value); + } + } + + awaitCatchEnter(node: Node, parent: Node) { + if (node.skip || parent.type !== 'AwaitBlock') { + return; + } + // Reset inits, this time only taking the error + // scope into account. + this.value.inits.clear(); + if (parent.error) { + this.handleScope(parent.error); + } + } + awaitLeave() { this.value = this.value.parent; } diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/expected.jsx b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/expected.jsx index 0cd41cc11..eadce747e 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/expected.jsx +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/expected.jsx @@ -1,13 +1,23 @@ <>{(hello) ? <> + {() => {/*Ωignore_startΩ*/const Ωhello=hello;/*Ωignore_endΩ*/let _$$p = (aPromise); __sveltets_awaitThen(_$$p, (hello) => {/*Ωignore_startΩ*/((Ωhello)) && /*Ωignore_endΩ*/<> + {hello} + }, () => {/*Ωignore_startΩ*/((hello)) && /*Ωignore_endΩ*/<> + {hello} + })}} + {() => {/*Ωignore_startΩ*/const Ωhello=hello;/*Ωignore_endΩ*/let _$$p = (aPromise); __sveltets_awaitThen(_$$p, (hi) => {/*Ωignore_startΩ*/((hello)) && /*Ωignore_endΩ*/<> + {hello} + }, (hello) => {/*Ωignore_startΩ*/((Ωhello)) && /*Ωignore_endΩ*/<> + {hello} + })}} {() => {/*Ωignore_startΩ*/const Ωhello=hello;/*Ωignore_endΩ*/let _$$p = (hello); __sveltets_awaitThen(_$$p, (hello) => {/*Ωignore_startΩ*/((Ωhello)) && /*Ωignore_endΩ*/<> {hello} {(hello) ? <> {() => {let _$$p = (aPromise); /*Ωignore_startΩ*/(((Ωhello))) && ((hello)) && /*Ωignore_endΩ*/<> {hello} - ; __sveltets_awaitThen(_$$p, () => {/*Ωignore_startΩ*/(((Ωhello))) && ((hello)) && /*Ωignore_endΩ*/<>})}} + ; __sveltets_awaitThen(_$$p, () => {<>})}} {() => {/*Ωignore_startΩ*/const ΩΩhello=hello;/*Ωignore_endΩ*/let _$$p = (aPromise); /*Ωignore_startΩ*/(((Ωhello))) && ((hello)) && /*Ωignore_endΩ*/<> {hello} - ; __sveltets_awaitThen(_$$p, () => {/*Ωignore_startΩ*/(((Ωhello))) && ((ΩΩhello)) && /*Ωignore_endΩ*/<>}, (hello) => {/*Ωignore_startΩ*/(((Ωhello))) && ((ΩΩhello)) && /*Ωignore_endΩ*/<> + ; __sveltets_awaitThen(_$$p, () => {<>}, (hello) => {/*Ωignore_startΩ*/(((Ωhello))) && ((ΩΩhello)) && /*Ωignore_endΩ*/<> {hello} })}} {() => {/*Ωignore_startΩ*/const ΩΩhello=hello;/*Ωignore_endΩ*/let _$$p = (x); __sveltets_awaitThen(_$$p, (hello) => {/*Ωignore_startΩ*/(((Ωhello))) && ((ΩΩhello)) && /*Ωignore_endΩ*/<> @@ -18,9 +28,9 @@ : <>} })}} {(hi && bye) ? <> - {() => {/*Ωignore_startΩ*/const Ωbye=bye,Ωhello=hello;/*Ωignore_endΩ*/let _$$p = (x); __sveltets_awaitThen(_$$p, (bye) => {/*Ωignore_startΩ*/(((Ωhello))) && ((hi && Ωbye)) && /*Ωignore_endΩ*/<> + {() => {/*Ωignore_startΩ*/const Ωbye=bye,Ωhello=hello;/*Ωignore_endΩ*/let _$$p = (x); __sveltets_awaitThen(_$$p, (bye) => {/*Ωignore_startΩ*/(((hello))) && ((hi && Ωbye)) && /*Ωignore_endΩ*/<> {bye} - }, (hello) => {/*Ωignore_startΩ*/(((Ωhello))) && ((hi && Ωbye)) && /*Ωignore_endΩ*/<> + }, (hello) => {/*Ωignore_startΩ*/(((Ωhello))) && ((hi && bye)) && /*Ωignore_endΩ*/<> {(hello) ? <> {hello} : <>} diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/input.svelte b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/input.svelte index 0b5c801c3..3b9dc7fdd 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/input.svelte +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block-shadowed/input.svelte @@ -1,4 +1,14 @@ {#if hello} + {#await aPromise then hello} + {hello} + {:catch} + {hello} + {/await} + {#await aPromise then hi} + {hello} + {:catch hello} + {hello} + {/await} {#await hello then hello} {hello} {#if hello} diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/expected.jsx b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/expected.jsx index 934fa1806..df23e0a45 100644 --- a/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/expected.jsx +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/if-nested-await-block/expected.jsx @@ -4,7 +4,7 @@ })}} {() => {let _$$p = (aPromise); /*Ωignore_startΩ*/((hello)) && /*Ωignore_endΩ*/<> {hello} - ; __sveltets_awaitThen(_$$p, () => {/*Ωignore_startΩ*/((hello)) && /*Ωignore_endΩ*/<>})}} + ; __sveltets_awaitThen(_$$p, () => {<>})}} {(hi && bye) ? <> {() => {let _$$p = (x); __sveltets_awaitThen(_$$p, (y) => {/*Ωignore_startΩ*/(((hello))) && ((hi && bye)) && /*Ωignore_endΩ*/<> {y} From ccf883a506ed43e58bafab4d702f61bdde696773 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 10 Mar 2021 08:50:57 +0100 Subject: [PATCH 17/17] cleanup --- packages/svelte2tsx/src/htmlxtojsx/nodes/slot.ts | 2 +- .../src/htmlxtojsx/nodes/template-scope.ts | 16 +++------------- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/slot.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/slot.ts index fd5713903..b079a3576 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/slot.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/slot.ts @@ -43,7 +43,7 @@ export function handleSlot( } else { str.remove(attr.start, attr.start + 'let:'.length); } - templateScope.add(attr.expression?.name || attr.name); + templateScope.inits.add(attr.expression?.name || attr.name); hasMoved = true; if (attr.expression) { //overwrite the = as a : diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/template-scope.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/template-scope.ts index c76c8e792..fb108339f 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/template-scope.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/template-scope.ts @@ -12,16 +12,6 @@ export class TemplateScope { this.parent = parent; } - addMany(inits: string[]) { - inits.forEach((item) => this.add(item)); - return this; - } - - add(name: string) { - this.inits.add(name); - return this; - } - child() { const child = new TemplateScope(this); return child; @@ -37,7 +27,7 @@ export class TemplateScopeManager { this.handleScope(node.context); } if (node.index) { - this.value.add(node.index); + this.value.inits.add(node.index); } } @@ -113,12 +103,12 @@ export class TemplateScopeManager { private handleScope(identifierDef: Node) { if (isIdentifier(identifierDef)) { - this.value.add(identifierDef.name); + this.value.inits.add(identifierDef.name); } if (isDestructuringPatterns(identifierDef)) { // the node object is returned as-it with no mutation const identifiers = extract_identifiers(identifierDef) as SvelteIdentifier[]; - this.value.addMany(identifiers.map((id) => id.name)); + identifiers.forEach((id) => this.value.inits.add(id.name)); } } }