Repository: sveltejs/language-tools
Pull request #1062 (2021-06-19 12:34:00)

(fix) better mapping for reactive statements
Map reactive statements/assignments a little different to ensure proper auto completions inside them.
Inspired by parts of #973 


Patch:
diff --git a/packages/svelte2tsx/src/svelte2tsx/nodes/ImplicitTopLevelNames.ts b/packages/svelte2tsx/src/svelte2tsx/nodes/ImplicitTopLevelNames.ts
index 280d31ec..f8d9c761 100644
--- a/packages/svelte2tsx/src/svelte2tsx/nodes/ImplicitTopLevelNames.ts
+++ b/packages/svelte2tsx/src/svelte2tsx/nodes/ImplicitTopLevelNames.ts
@@ -4,15 +4,53 @@ import {
     isParenthesizedObjectOrArrayLiteralExpression,
     getNamesFromLabeledStatement
 } from '../utils/tsAst';
+import { preprendStr } from '../../utils/magic-string';
 
 export class ImplicitTopLevelNames {
     private map = new Set<ts.LabeledStatement>();
 
+    constructor(private str: MagicString, private astOffset: number) {}
+
     add(node: ts.LabeledStatement) {
         this.map.add(node);
     }
 
-    modifyCode(rootVariables: Set<string>, astOffset: number, str: MagicString) {
+    handleReactiveStatement(
+        node: ts.LabeledStatement,
+        binaryExpression: ts.BinaryExpression | undefined
+    ): void {
+        if (binaryExpression) {
+            this.wrapExpressionWithInvalidate(binaryExpression.right);
+        } else {
+            const start = node.getStart() + this.astOffset;
+            const end = node.getEnd() + this.astOffset;
+
+            this.str.prependLeft(start, ';() => {');
+            preprendStr(this.str, end, '}');
+        }
+    }
+
+    private wrapExpressionWithInvalidate(expression: ts.Expression | undefined): void {
+        if (!expression) {
+            return;
+        }
+
+        const start = expression.getStart() + this.astOffset;
+        const end = expression.getEnd() + this.astOffset;
+
+        // () => ({})
+        if (ts.isObjectLiteralExpression(expression)) {
+            this.str.appendLeft(start, '(');
+            this.str.appendRight(end, ')');
+        }
+
+        this.str.prependLeft(start, '__sveltets_invalidate(() => ');
+        preprendStr(this.str, end, ')');
+        // Not adding ';' at the end because right now this function is only invoked
+        // in situations where there is a line break of ; guaranteed to be present (else the code is invalid)
+    }
+
+    modifyCode(rootVariables: Set<string>) {
         for (const node of this.map.values()) {
             const names = getNamesFromLabeledStatement(node);
             if (names.length === 0) {
@@ -24,13 +62,13 @@ export class ImplicitTopLevelNames {
 
             if (this.hasOnlyImplicitTopLevelNames(names, implicitTopLevelNames)) {
                 // remove '$:' label
-                str.remove(pos + astOffset, pos + astOffset + 2);
-                str.prependRight(pos + astOffset, 'let ');
+                this.str.remove(pos + this.astOffset, pos + this.astOffset + 2);
+                this.str.prependRight(pos + this.astOffset, 'let ');
 
-                this.removeBracesFromParenthizedExpression(node, astOffset, str);
+                this.removeBracesFromParenthizedExpression(node);
             } else {
                 implicitTopLevelNames.forEach((name) => {
-                    str.prependRight(pos + astOffset, `let ${name};\n`);
+                    this.str.prependRight(pos + this.astOffset, `let ${name};\n`);
                 });
             }
         }
@@ -40,11 +78,7 @@ export class ImplicitTopLevelNames {
         return names.length === implicitTopLevelNames.length;
     }
 
-    private removeBracesFromParenthizedExpression(
-        node: ts.LabeledStatement,
-        astOffset: number,
-        str: MagicString
-    ) {
+    private removeBracesFromParenthizedExpression(node: ts.LabeledStatement) {
         // If expression is of type `$: ({a} = b);`,
         // remove the surrounding braces so that the transformation
         // to `let {a} = b;` produces valid code.
@@ -52,10 +86,12 @@ export class ImplicitTopLevelNames {
             ts.isExpressionStatement(node.statement) &&
             isParenthesizedObjectOrArrayLiteralExpression(node.statement.expression)
         ) {
-            const start = node.statement.expression.getStart() + astOffset;
-            str.overwrite(start, start + 1, '', { contentOnly: true });
-            const end = node.statement.expression.getEnd() + astOffset - 1;
-            str.overwrite(end, end + 1, '', { contentOnly: true });
+            const start = node.statement.expression.getStart() + this.astOffset;
+            this.str.overwrite(start, start + 1, '', { contentOnly: true });
+            const end = node.statement.expression.getEnd() + this.astOffset - 1;
+            // We need to keep the `)` of the "wrap with invalidate" expression above.
+            // We overwrite the same range so it's needed.
+            this.str.overwrite(end, end + 1, ')', { contentOnly: true });
         }
     }
 }
diff --git a/packages/svelte2tsx/src/svelte2tsx/processInstanceScriptContent.ts b/packages/svelte2tsx/src/svelte2tsx/processInstanceScriptContent.ts
index c2db71d7..ecc0e5cb 100644
--- a/packages/svelte2tsx/src/svelte2tsx/processInstanceScriptContent.ts
+++ b/packages/svelte2tsx/src/svelte2tsx/processInstanceScriptContent.ts
@@ -53,7 +53,7 @@ export function processInstanceScriptContent(
     const getters = new Set<string>();
     const generics = new Generics(str, astOffset);
 
-    const implicitTopLevelNames = new ImplicitTopLevelNames();
+    const implicitTopLevelNames = new ImplicitTopLevelNames(str, astOffset);
     let uses$$props = false;
     let uses$$restProps = false;
     let uses$$slots = false;
@@ -332,26 +332,6 @@ export function processInstanceScriptContent(
         });
     };
 
-    const wrapExpressionWithInvalidate = (expression: ts.Expression | undefined) => {
-        if (!expression) {
-            return;
-        }
-
-        const start = expression.getStart() + astOffset;
-        const end = expression.getEnd() + astOffset;
-
-        // () => ({})
-        if (ts.isObjectLiteralExpression(expression)) {
-            str.appendLeft(start, '(');
-            str.appendRight(end, ')');
-        }
-
-        str.prependLeft(start, '__sveltets_invalidate(() => ');
-        str.appendRight(end, ')');
-        // Not adding ';' at the end because right now this function is only invoked
-        // in situations where there is a line break of ; guaranteed to be present (else the code is invalid)
-    };
-
     const walk = (node: ts.Node, parent: ts.Node) => {
         type onLeaveCallback = () => void;
         const onLeaveCallbacks: onLeaveCallback[] = [];
@@ -489,14 +469,8 @@ export function processInstanceScriptContent(
             if (binaryExpression) {
                 implicitTopLevelNames.add(node);
                 implicitStoreValues.addReactiveDeclaration(node);
-                wrapExpressionWithInvalidate(binaryExpression.right);
-            } else {
-                const start = node.getStart() + astOffset;
-                const end = node.getEnd() + astOffset;
-
-                str.prependLeft(start, ';() => {');
-                str.appendRight(end, '}');
             }
+            implicitTopLevelNames.handleReactiveStatement(node, binaryExpression);
         }
 
         // Defensively call function (checking for undefined) because it got added only recently (TS 4.0)
@@ -518,7 +492,7 @@ export function processInstanceScriptContent(
     pendingStoreResolutions.map(resolveStore);
 
     // declare implicit reactive variables we found in the script
-    implicitTopLevelNames.modifyCode(rootScope.declared, astOffset, str);
+    implicitTopLevelNames.modifyCode(rootScope.declared);
     implicitStoreValues.modifyCode(astOffset, str);
 
     const firstImport = tsAst.statements
diff --git a/packages/svelte2tsx/src/utils/magic-string.ts b/packages/svelte2tsx/src/utils/magic-string.ts
new file mode 100644
index 00000000..6f305311
--- /dev/null
+++ b/packages/svelte2tsx/src/utils/magic-string.ts
@@ -0,0 +1,10 @@
+import MagicString from 'magic-string';
+
+/**
+ * Prepends a string at the given index in a way that the source map maps the appended string
+ * to the given character, not the previous character (as MagicString's other methods would).
+ */
+export function preprendStr(str: MagicString, index: number, toAppend: string): MagicString {
+    str.overwrite(index, index + 1, toAppend + str.original.charAt(index), { contentOnly: true });
+    return str;
+}


Test patch:
diff --git a/packages/language-server/test/plugins/typescript/features/CompletionProvider.test.ts b/packages/language-server/test/plugins/typescript/features/CompletionProvider.test.ts
index e7367380..1ff36247 100644
--- a/packages/language-server/test/plugins/typescript/features/CompletionProvider.test.ts
+++ b/packages/language-server/test/plugins/typescript/features/CompletionProvider.test.ts
@@ -795,6 +795,26 @@ describe('CompletionProviderImpl', () => {
             `/**${newLine} * $0${newLine} * @param parameter1${newLine} */`
         );
     });
+
+    it('shows completions in reactive statement', async () => {
+        const { completionProvider, document } = setup('completions-in-reactive-statement.svelte');
+
+        await checkCompletion(Position.create(9, 13));
+        await checkCompletion(Position.create(10, 16));
+        await checkCompletion(Position.create(11, 14));
+        await checkCompletion(Position.create(13, 17));
+        await checkCompletion(Position.create(14, 20));
+        await checkCompletion(Position.create(15, 18));
+
+        async function checkCompletion(position: Position) {
+            const completions = await completionProvider.getCompletions(document, position, {
+                triggerKind: CompletionTriggerKind.Invoked
+            });
+            assert.strictEqual(completions?.items.length, 1);
+            const item = completions?.items?.[0];
+            assert.strictEqual(item?.label, 'abc');
+        }
+    }).timeout(4000);
 });
 
 function harmonizeNewLines(input?: string) {
diff --git a/packages/language-server/test/plugins/typescript/testfiles/completions/completions-in-reactive-statement.svelte b/packages/language-server/test/plugins/typescript/testfiles/completions/completions-in-reactive-statement.svelte
new file mode 100644
index 00000000..454dc6af
--- /dev/null
+++ b/packages/language-server/test/plugins/typescript/testfiles/completions/completions-in-reactive-statement.svelte
@@ -0,0 +1,17 @@
+<script lang="ts">
+    import { writable } from "svelte/store";
+
+    const prop = { abc: true };
+
+    function fun() { return prop; }
+    const variable = prop;
+    const store = writable(prop);
+
+    $: fun().;
+    $: variable.;
+    $: $store.;
+
+    $: a = fun().;
+    $: b = variable.;
+    $: c = $store.;
+</script>
diff --git a/packages/svelte2tsx/test/sourcemaps/samples/large-sample-1/mappings.jsx b/packages/svelte2tsx/test/sourcemaps/samples/large-sample-1/mappings.jsx
index 91c76096..a67b3b8e 100644
--- a/packages/svelte2tsx/test/sourcemaps/samples/large-sample-1/mappings.jsx
+++ b/packages/svelte2tsx/test/sourcemaps/samples/large-sample-1/mappings.jsx
@@ -161,7 +161,7 @@ s
 //----------------------------------------------------------------------------------------------------------------------------------------------------
     ;() => {$: if (scrollable) chapter, scrollable.scrollTo(0, 0);}                                                                                   {/**
    ╚;()•=>•{$:•if•(scrollable)•chapter,•scrollable.scrollTo(0,•0);}↲    [generated] line 67                                                           
-   ╚        $:•if•(scrollable)•chapter,•scrollable.scrollTo(0,•0); ↲                                                                                  
+   ╚        $:•if•(scrollable)•chapter,•scrollable.scrollTo(0,•0);↲                                                                                   
    ╚$:•if•(scrollable)•chapter,•scrollable.scrollTo(0,•0);↲             [original] line 61                                                            
 ------------------------------------------------------------------------------------------------------------------------------------------------------ */}
 
@@ -172,13 +172,13 @@ s
 ------------------------------------------------------------------------------------------------------------------------------------------------------ */}
     let  selected = __sveltets_invalidate(() => lookup.get(slug));                                                                                    {/**
    ╚let••selected•=•__sveltets_invalidate(()•=>•lookup.get(slug));↲    [generated] line 73                                                            
-   ╚    •selected•=•                            lookup.get(slug) ;↲                                                                                   
+   ╚    •selected•=•                            lookup.get(slug); ↲                                                                                   
    ╚  •selected•=•lookup.get(slug);↲                                                                                                                  
    ╚$:•selected•=•lookup.get(slug);↲                                   [original] line 67                                                             
 ------------------------------------------------------------------------------------------------------------------------------------------------------ */}
     let  improve_link = __sveltets_invalidate(() => `${tutorial_repo_link}/${selected.chapter.section_dir}/${selected.chapter.chapter_dir}`);         {/**
    ╚let••improve_link•=•__sveltets_invalidate(()•=>•`${tutorial_repo_link}/${selected.chapter.section_dir}/${selected.chapter.chapter_dir}`);↲    [generated] line 74
-   ╚    •improve_link•=•                            `${tutorial_repo_link}/${selected.chapter.section_dir}/${selected.chapter.chapter_dir}` ;↲        
+   ╚    •improve_link•=•                            `${tutorial_repo_link}/${selected.chapter.section_dir}/${selected.chapter.chapter_dir}`; ↲        
    ╚  •improve_link•=•`${tutorial_repo_link}/${selected.chapter.section_dir}/${selected.chapter.chapter_dir}`;↲                                       
    ╚$:•improve_link•=•`${tutorial_repo_link}/${selected.chapter.section_dir}/${selected.chapter.chapter_dir}`;↲                                   [original] line 68 
 ------------------------------------------------------------------------------------------------------------------------------------------------------ */}
@@ -202,14 +202,13 @@ s
 ------------------------------------------------------------------------------------------------------------------------------------------------------ */}
     }}                                                                                                                                                {/**
    ╚}}↲    [generated] line 87                                                                                                                        
-   ╚} ↲                                                                                                                                               
    ╚}↲     [original] line 81                                                                                                                         
 ------------------------------------------------------------------------------------------------------------------------------------------------------ */}
                                                                                                                                                       {/**
 ------------------------------------------------------------------------------------------------------------------------------------------------------ */}
     let  mobile = __sveltets_invalidate(() => width < 768);                                                                                           {/**
    ╚let••mobile•=•__sveltets_invalidate(()•=>•width•<•768);↲    [generated] line 89                                                                   
-   ╚    •mobile•=•                            width•<•768 ;↲                                                                                          
+   ╚    •mobile•=•                            width•<•768; ↲                                                                                          
    ╚  •mobile•=•width•<•768;↲                                                                                                                         
    ╚$:•mobile•=•width•<•768;↲                                   [original] line 83                                                                    
 ------------------------------------------------------------------------------------------------------------------------------------------------------ */}
diff --git a/packages/svelte2tsx/test/sourcemaps/samples/let/mappings.jsx b/packages/svelte2tsx/test/sourcemaps/samples/let/mappings.jsx
index a6822847..a77d866b 100644
--- a/packages/svelte2tsx/test/sourcemaps/samples/let/mappings.jsx
+++ b/packages/svelte2tsx/test/sourcemaps/samples/let/mappings.jsx
@@ -14,7 +14,7 @@
 ------------------------------------------------------------------------------------------------------------------------------------------------------ */}
     let  selected = __sveltets_invalidate(() => lookup.get(slug));                                                                                    {/**
 ••••let••selected•=•__sveltets_invalidate(()•=>•lookup.get(slug));↲    [generated] line 4                                                             
-••••    •selected•=•                            lookup.get(slug) ;↲                                                                                   
+••••    •selected•=•                            lookup.get(slug); ↲                                                                                   
 ••••  •selected•=•lookup.get(slug);↲                                                                                                                  
 ••••$:•selected•=•lookup.get(slug);↲                                   [original] line 2                                                              
 ------------------------------------------------------------------------------------------------------------------------------------------------------ */}
diff --git a/packages/svelte2tsx/test/sourcemaps/samples/reactive-statements/mappings.jsx b/packages/svelte2tsx/test/sourcemaps/samples/reactive-statements/mappings.jsx
index 16ba096a..3d243b41 100644
--- a/packages/svelte2tsx/test/sourcemaps/samples/reactive-statements/mappings.jsx
+++ b/packages/svelte2tsx/test/sourcemaps/samples/reactive-statements/mappings.jsx
@@ -20,24 +20,24 @@
 ------------------------------------------------------------------------------------------------------------------------------------------------------ */}
     let  foo = __sveltets_invalidate(() => prop);                                                                                                     {/**
    ╚let••foo•=•__sveltets_invalidate(()•=>•prop);↲    [generated] line 5                                                                              
-   ╚    •foo•=•                            prop ;↲                                                                                                    
+   ╚    •foo•=•                            prop; ↲                                                                                                    
    ╚  •foo•=•prop;↲                                                                                                                                   
    ╚$:•foo•=•prop;↲                                   [original] line 3                                                                               
 ------------------------------------------------------------------------------------------------------------------------------------------------------ */}
     let  bar = __sveltets_invalidate(() => (__sveltets_store_get(prop), $prop));                                                                      {/**
    ╚let••bar•=•__sveltets_invalidate(()•=>•(__sveltets_store_get(prop),•$prop));↲    [generated] line 6                                               
-   ╚    •bar•=•                            $                     prop          ;↲                                                                     
+   ╚    •bar•=•                            $                     prop         ; ↲                                                                     
    ╚  •bar•=•$prop;↲                                                                                                                                  
    ╚$:•bar•=•$prop;↲                                                                 [original] line 4                                                
 ------------------------------------------------------------------------------------------------------------------------------------------------------ */}
     ;() => {$: if (bar) ++foo}                                                                                                                        {/**
    ╚;()•=>•{$:•if•(bar)•++foo}↲    [generated] line 7                                                                                                 
-   ╚        $:•if•(bar)•++foo ↲                                                                                                                       
+   ╚        $:•if•(bar)•++foo↲                                                                                                                        
    ╚$:•if•(bar)•++foo↲             [original] line 5                                                                                                  
 ------------------------------------------------------------------------------------------------------------------------------------------------------ */}
     ;() => {$: { if (foo) bar(); }}                                                                                                                   {/**
    ╚;()•=>•{$:•{•if•(foo)•bar();•}}↲    [generated] line 8                                                                                            
-   ╚        $:•{•if•(foo)•bar();•} ↲                                                                                                                  
+   ╚        $:•{•if•(foo)•bar();•}↲                                                                                                                   
    ╚$:•{•if•(foo)•bar();•}↲             [original] line 6                                                                                             
 ------------------------------------------------------------------------------------------------------------------------------------------------------ */}
 ;                                                                                                                                                     {/**
diff --git a/packages/svelte2tsx/test/sourcemaps/samples/reserved-variables/mappings.jsx b/packages/svelte2tsx/test/sourcemaps/samples/reserved-variables/mappings.jsx
index c525e443..f8a29e0b 100644
--- a/packages/svelte2tsx/test/sourcemaps/samples/reserved-variables/mappings.jsx
+++ b/packages/svelte2tsx/test/sourcemaps/samples/reserved-variables/mappings.jsx
@@ -18,7 +18,7 @@
 ------------------------------------------------------------------------------------------------------------------------------------------------------ */}
     let  foo = __sveltets_invalidate(() => ({...$$slots, ...$$restProps, ...$$props}));                                                               {/**
    ╚let••foo•=•__sveltets_invalidate(()•=>•({...$$slots,•...$$restProps,•...$$props}));↲    [generated] line 7                                        
-   ╚    •foo•=•                             {...$$slots,•...$$restProps,•...$$props}  ;↲                                                              
+   ╚    •foo•=•                             {...$$slots,•...$$restProps,•...$$props} ; ↲                                                              
    ╚  •foo•=•{...$$slots,•...$$restProps,•...$$props};↲                                                                                               
    ╚$:•foo•=•{...$$slots,•...$$restProps,•...$$props};↲                                     [original] line 5                                         
 ------------------------------------------------------------------------------------------------------------------------------------------------------ */}