From d43b7d940ce69837bfb742aaae63cc6e6b7e3a50 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 19 Mar 2021 09:35:55 +0100 Subject: [PATCH 1/2] (fix) move store decl from import into render fn Don't append the store declaration of imports that are stores right after the import. Instead, append it to the start of the render function. This way, imports are grouped without these declarations at the top, which makes "organize imports" behave correctly again and not put ignore-comments into the edits. #879 --- packages/svelte2tsx/src/svelte2tsx/index.ts | 7 +++-- .../svelte2tsx/nodes/ImplicitStoreValues.ts | 28 +++++++++---------- .../samples/store-from-module/expected.tsx | 4 +-- .../samples/store-import/expected.tsx | 8 +++--- 4 files changed, 25 insertions(+), 22 deletions(-) diff --git a/packages/svelte2tsx/src/svelte2tsx/index.ts b/packages/svelte2tsx/src/svelte2tsx/index.ts index 22ca36503..f2d73ed7c 100644 --- a/packages/svelte2tsx/src/svelte2tsx/index.ts +++ b/packages/svelte2tsx/src/svelte2tsx/index.ts @@ -455,7 +455,10 @@ export function svelte2tsx( } } - const implicitStoreValues = new ImplicitStoreValues(resolvedStores); + const renderFunctionStart = scriptTag + ? str.original.lastIndexOf('>', scriptTag.content.start) + 1 + : instanceScriptTarget; + const implicitStoreValues = new ImplicitStoreValues(resolvedStores, renderFunctionStart); //move the instance script and process the content let exportedNames = new ExportedNames(); let getters = new Set(); @@ -492,7 +495,7 @@ export function svelte2tsx( processModuleScriptTag( str, moduleScriptTag, - new ImplicitStoreValues(implicitStoreValues.getAccessedStores()) + new ImplicitStoreValues(implicitStoreValues.getAccessedStores(), renderFunctionStart) ); } diff --git a/packages/svelte2tsx/src/svelte2tsx/nodes/ImplicitStoreValues.ts b/packages/svelte2tsx/src/svelte2tsx/nodes/ImplicitStoreValues.ts index 910113422..cd6d0d36f 100644 --- a/packages/svelte2tsx/src/svelte2tsx/nodes/ImplicitStoreValues.ts +++ b/packages/svelte2tsx/src/svelte2tsx/nodes/ImplicitStoreValues.ts @@ -20,7 +20,7 @@ export class ImplicitStoreValues { public addReactiveDeclaration = this.reactiveDeclarations.push.bind(this.reactiveDeclarations); public addImportStatement = this.importStatements.push.bind(this.importStatements); - constructor(storesResolvedInTemplate: string[] = []) { + constructor(storesResolvedInTemplate: string[] = [], private renderFunctionStart: number) { storesResolvedInTemplate.forEach(this.addStoreAcess); } @@ -37,9 +37,7 @@ export class ImplicitStoreValues { this.attachStoreValueDeclarationToReactiveAssignment(node, astOffset, str) ); - this.importStatements - .filter(({ name }) => name && this.accessedStores.has(name.getText())) - .forEach((node) => this.attachStoreValueDeclarationToImport(node, astOffset, str)); + this.attachStoreValueDeclarationOfImportsToRenderFn(str); } public getAccessedStores(): string[] { @@ -89,17 +87,19 @@ export class ImplicitStoreValues { str.appendRight(endPos, storeDeclarations); } - private attachStoreValueDeclarationToImport( - node: ts.ImportClause | ts.ImportSpecifier, - astOffset: number, - str: MagicString - ) { - const storeName = node.name.getText(); - const storeDeclaration = surroundWithIgnoreComments(this.createStoreDeclaration(storeName)); - const importStatement = ts.isImportClause(node) ? node.parent : node.parent.parent.parent; - const endPos = importStatement.getEnd() + astOffset; + private attachStoreValueDeclarationOfImportsToRenderFn(str: MagicString) { + const storeNames = this.importStatements + .filter(({ name }) => name && this.accessedStores.has(name.getText())) + .map(({ name }) => name.getText()); + if (!storeNames.length) { + return; + } + + const storeDeclarations = surroundWithIgnoreComments( + this.createStoreDeclarations(storeNames) + ); - str.appendRight(endPos, storeDeclaration); + str.appendRight(this.renderFunctionStart, storeDeclarations); } private createStoreDeclarations(storeNames: string[]): string { diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/store-from-module/expected.tsx b/packages/svelte2tsx/test/svelte2tsx/samples/store-from-module/expected.tsx index b65557f52..81360a395 100644 --- a/packages/svelte2tsx/test/svelte2tsx/samples/store-from-module/expected.tsx +++ b/packages/svelte2tsx/test/svelte2tsx/samples/store-from-module/expected.tsx @@ -1,10 +1,10 @@ /// <>; - import {store1, store2} from './store';/*Ωignore_startΩ*/;let $store1 = __sveltets_store_get(store1);/*Ωignore_endΩ*//*Ωignore_startΩ*/;let $store2 = __sveltets_store_get(store2);/*Ωignore_endΩ*/ + import {store1, store2} from './store'; const store3 = writable('')/*Ωignore_startΩ*/;let $store3 = __sveltets_store_get(store3);/*Ωignore_endΩ*/; const store4 = writable('')/*Ωignore_startΩ*/;let $store4 = __sveltets_store_get(store4);/*Ωignore_endΩ*/; ;<>;function render() { - +/*Ωignore_startΩ*/;let $store1 = __sveltets_store_get(store1);;let $store2 = __sveltets_store_get(store2);/*Ωignore_endΩ*/ ;(__sveltets_store_get(store1), $store1); ;(__sveltets_store_get(store3), $store3); ; diff --git a/packages/svelte2tsx/test/svelte2tsx/samples/store-import/expected.tsx b/packages/svelte2tsx/test/svelte2tsx/samples/store-import/expected.tsx index 2233278bb..680fbbe20 100644 --- a/packages/svelte2tsx/test/svelte2tsx/samples/store-import/expected.tsx +++ b/packages/svelte2tsx/test/svelte2tsx/samples/store-import/expected.tsx @@ -4,10 +4,10 @@ import storeA from './store'; import { storeB } from './store'; import { storeB as storeC } from './store'; function render() { - - /*Ωignore_startΩ*/;let $storeA = __sveltets_store_get(storeA);/*Ωignore_endΩ*/ - /*Ωignore_startΩ*/;let $storeB = __sveltets_store_get(storeB);/*Ωignore_endΩ*/ - /*Ωignore_startΩ*/;let $storeC = __sveltets_store_get(storeC);/*Ωignore_endΩ*/ +/*Ωignore_startΩ*/;let $storeA = __sveltets_store_get(storeA);;let $storeB = __sveltets_store_get(storeB);;let $storeC = __sveltets_store_get(storeC);/*Ωignore_endΩ*/ + + + ; () => (<> From d975c5086060fac2b5ba2beba3bf0e5fa9fbbe25 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 19 Mar 2021 09:41:30 +0100 Subject: [PATCH 2/2] test --- .../features/CodeActionsProvider.test.ts | 88 +++++++++++++++++++ .../organize-imports-module-store.svelte | 13 +++ 2 files changed, 101 insertions(+) create mode 100644 packages/language-server/test/plugins/typescript/testfiles/organize-imports-module-store.svelte diff --git a/packages/language-server/test/plugins/typescript/features/CodeActionsProvider.test.ts b/packages/language-server/test/plugins/typescript/features/CodeActionsProvider.test.ts index 13f701011..e6fa7c1cf 100644 --- a/packages/language-server/test/plugins/typescript/features/CodeActionsProvider.test.ts +++ b/packages/language-server/test/plugins/typescript/features/CodeActionsProvider.test.ts @@ -316,6 +316,94 @@ describe('CodeActionsProvider', () => { ]); }); + it('organizes imports with module script and store', async () => { + const { provider, document } = setup('organize-imports-module-store.svelte'); + + const codeActions = await provider.getCodeActions( + document, + Range.create(Position.create(1, 4), Position.create(1, 5)), // irrelevant + { + diagnostics: [], + only: [CodeActionKind.SourceOrganizeImports] + } + ); + (codeActions[0]?.edit?.documentChanges?.[0])?.edits.forEach( + (edit) => (edit.newText = harmonizeNewLines(edit.newText)) + ); + + assert.deepStrictEqual(codeActions, [ + { + edit: { + documentChanges: [ + { + edits: [ + { + newText: + "import { _,_d } from 'svelte-i18n';\nimport { _e } from 'svelte-i18n1';\n", + range: { + end: { + character: 0, + line: 2 + }, + start: { + character: 2, + line: 1 + } + } + }, + { + newText: '', + range: { + end: { + character: 2, + line: 6 + }, + start: { + character: 2, + line: 5 + } + } + }, + { + newText: '', + range: { + end: { + character: 2, + line: 7 + }, + start: { + character: 2, + line: 6 + } + } + }, + { + newText: '', + range: { + end: { + character: 37, + line: 7 + }, + start: { + character: 2, + line: 7 + } + } + } + ], + textDocument: { + uri: getUri('organize-imports-module-store.svelte'), + version: 0 + } + } + ] + }, + kind: CodeActionKind.SourceOrganizeImports, + title: 'Organize Imports' + } + ]); + }); + it('should do extract into const refactor', async () => { const { provider, document } = setup('codeactions.svelte'); diff --git a/packages/language-server/test/plugins/typescript/testfiles/organize-imports-module-store.svelte b/packages/language-server/test/plugins/typescript/testfiles/organize-imports-module-store.svelte new file mode 100644 index 000000000..f5a2b0070 --- /dev/null +++ b/packages/language-server/test/plugins/typescript/testfiles/organize-imports-module-store.svelte @@ -0,0 +1,13 @@ + + + + +

{$_('test')}

+

{$_d('test')}

+

{$_e('test')}