Avoid duplication when organizing imports (#391)

Organizing imports sometimes duplicates them. This fixes that by including a trailing newline right after the ESM nodes in the virtual code ranges.
mdx-js · Feb 2, 2024 · 2b29d7a · 2b29d7a
1 parent 3069af7
commit 2b29d7a
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 29 deletions.
diff --git a/.changeset/unlucky-tools-kiss.md b/.changeset/unlucky-tools-kiss.md
@@ -0,0 +1,7 @@
+---
+"@mdx-js/language-service": patch
+"@mdx-js/language-server": patch
+"vscode-mdx": patch
+---
+
+Avoid duplication when organizing imports.
diff --git a/packages/language-server/test/completion.test.js b/packages/language-server/test/completion.test.js
@@ -93,7 +93,7 @@ test('support completion in JSX', async () => {
       original: {
         data: {
           fileName: fixturePath('node16/completion.mdx'),
-          offset: 118,
+          offset: 119,
           originalItem: {name: 'Boolean'},
           uri: fixtureUri('node16/completion.mdx?virtualCodeId=jsx')
         }
@@ -113,7 +113,7 @@ test('support completion in JSX', async () => {
     commitCharacters: ['.', ',', ';', '('],
     data: {
       fileName: fixturePath('node16/completion.mdx'),
-      offset: 118,
+      offset: 119,
       originalItem: {name: 'Boolean'},
       uri: fixtureUri('node16/completion.mdx?virtualCodeId=jsx')
     },

diff --git a/packages/language-service/lib/virtual-code.js b/packages/language-service/lib/virtual-code.js
@@ -117,14 +117,38 @@ function visit(node, onEnter, onExit) {
  *   The start offset in the original source code.
  * @param {number} endOffset
  *   The end offset in the original source code.
+ * @param {boolean} [includeNewline]
+ *  If true, and the source range is followed directly by a newline, extend the
+ *  end offset to include that newline.
  * @returns {string}
  *   The updated generated content.
  */
-function addOffset(mapping, source, generated, startOffset, endOffset) {
+function addOffset(
+  mapping,
+  source,
+  generated,
+  startOffset,
+  endOffset,
+  includeNewline
+) {
   if (startOffset === endOffset) {
     return generated
   }
 
+  if (includeNewline) {
+    const LF = 10
+    const CR = 13
+    // eslint-disable-next-line unicorn/prefer-code-point
+    const charCode = source.charCodeAt(endOffset)
+    if (charCode === LF) {
+      endOffset += 1
+    }
+    // eslint-disable-next-line unicorn/prefer-code-point
+    else if (charCode === CR && source.charCodeAt(endOffset + 1) === LF) {
+      endOffset += 2
+    }
+  }
+
   const length = endOffset - startOffset
   const previousSourceOffset = mapping.sourceOffsets.at(-1)
   const previousGeneratedOffset = mapping.generatedOffsets.at(-1)
@@ -196,7 +220,7 @@ function processExports(mdx, node, mapping, esm) {
   const body = node.data?.estree?.body
 
   if (!body?.length) {
-    return addOffset(mapping, mdx, esm, start, end) + '\n'
+    return addOffset(mapping, mdx, esm, start, end, true)
   }
 
   for (const child of body) {
@@ -206,14 +230,14 @@ function processExports(mdx, node, mapping, esm) {
         esm += layoutJsDoc(propsName)
       }
 
-      esm =
-        addOffset(
-          mapping,
-          mdx,
-          esm + '\nconst MDXLayout = ',
-          child.declaration.start,
-          child.end
-        ) + '\n'
+      esm = addOffset(
+        mapping,
+        mdx,
+        esm + '\nconst MDXLayout = ',
+        child.declaration.start,
+        child.end,
+        true
+      )
       continue
     }
 
@@ -228,20 +252,21 @@ function processExports(mdx, node, mapping, esm) {
               ? specifier.end
               : mdx.indexOf(',', specifier.end) + 1
           return (
-            addOffset(mapping, mdx, esm, nextPosition, end) +
+            addOffset(mapping, mdx, esm, nextPosition, end, true) +
             '\nimport {' +
             specifier.exported.name +
             ' as MDXLayout} from ' +
-            JSON.stringify(child.source.value)
+            JSON.stringify(child.source.value) +
+            '\n'
           )
         }
       }
     }
 
-    esm = addOffset(mapping, mdx, esm, child.start, child.end) + '\n'
+    esm = addOffset(mapping, mdx, esm, child.start, child.end, true)
   }
 
-  return esm
+  return esm + '\n'
 }
 
 /**

diff --git a/packages/language-service/test/language-module.js b/packages/language-service/test/language-module.js
@@ -47,7 +47,7 @@ test('create virtual code w/ mdxjsEsm', () => {
         {
           sourceOffsets: [0],
           generatedOffsets: [51],
-          lengths: [34],
+          lengths: [35],
           data: {
             completion: true,
             format: false,
@@ -63,6 +63,7 @@ test('create virtual code w/ mdxjsEsm', () => {
         '@jsxImportSource react */',
         'import {Planet} from "./Planet.js"',
         '',
+        '',
         '/**',
         ' * @deprecated',
         ' *   Do not use.',
@@ -149,7 +150,7 @@ test('create virtual code w/o MDX layout in case of named re-export', () => {
         {
           sourceOffsets: [0],
           generatedOffsets: [51],
-          lengths: [33],
+          lengths: [34],
           data: {
             completion: true,
             format: false,
@@ -165,6 +166,7 @@ test('create virtual code w/o MDX layout in case of named re-export', () => {
         '@jsxImportSource react */',
         'export {named} from "./Layout.js"',
         '',
+        '',
         '/**',
         ' * @deprecated',
         ' *   Do not use.',
@@ -251,7 +253,7 @@ test('create virtual code w/ MDX layout in case of default re-export', () => {
         {
           sourceOffsets: [0, 15],
           generatedOffsets: [51, 59],
-          lengths: [8, 20],
+          lengths: [8, 21],
           data: {
             completion: true,
             format: false,
@@ -266,7 +268,9 @@ test('create virtual code w/ MDX layout in case of default re-export', () => {
         '/* @jsxRuntime automatic',
         '@jsxImportSource react */',
         'export {} from "./Layout.js"',
+        '',
         'import {default as MDXLayout} from "./Layout.js"',
+        '',
         '/**',
         ' * @deprecated',
         ' *   Do not use.',
@@ -356,7 +360,7 @@ test('create virtual code w/ MDX layout in case of named and default re-export',
         {
           sourceOffsets: [0, 22],
           generatedOffsets: [51, 66],
-          lengths: [15, 20],
+          lengths: [15, 21],
           data: {
             completion: true,
             format: false,
@@ -371,7 +375,9 @@ test('create virtual code w/ MDX layout in case of named and default re-export',
         '/* @jsxRuntime automatic',
         '@jsxImportSource react */',
         'export {named, } from "./Layout.js"',
+        '',
         'import {default as MDXLayout} from "./Layout.js"',
+        '',
         '/**',
         ' * @deprecated',
         ' *   Do not use.',
@@ -461,7 +467,7 @@ test('create virtual code w/ MDX layout in case of default and named re-export',
         {
           sourceOffsets: [0, 16],
           generatedOffsets: [51, 59],
-          lengths: [8, 26],
+          lengths: [8, 27],
           data: {
             completion: true,
             format: false,
@@ -476,7 +482,9 @@ test('create virtual code w/ MDX layout in case of default and named re-export',
         '/* @jsxRuntime automatic',
         '@jsxImportSource react */',
         'export { named} from "./Layout.js"',
+        '',
         'import {default as MDXLayout} from "./Layout.js"',
+        '',
         '/**',
         ' * @deprecated',
         ' *   Do not use.',
@@ -563,7 +571,7 @@ test('create virtual code w/ MDX layout in case of a default exported arrow func
         {
           sourceOffsets: [15],
           generatedOffsets: [721],
-          lengths: [8],
+          lengths: [9],
           data: {
             completion: true,
             format: false,
@@ -593,6 +601,7 @@ test('create virtual code w/ MDX layout in case of a default exported arrow func
         ' */',
         'const MDXLayout = () => {}',
         '',
+        '',
         '/**',
         ' * @deprecated',
         ' *   Do not use.',
@@ -682,7 +691,7 @@ test('create virtual code w/ MDX layout in case of a default exported function d
         {
           sourceOffsets: [15],
           generatedOffsets: [721],
-          lengths: [23],
+          lengths: [24],
           data: {
             completion: true,
             format: false,
@@ -712,6 +721,7 @@ test('create virtual code w/ MDX layout in case of a default exported function d
         ' */',
         'const MDXLayout = function MDXLayout() {}',
         '',
+        '',
         '/**',
         ' * @deprecated',
         ' *   Do not use.',
@@ -798,7 +808,7 @@ test('create virtual code w/ MDX layout in case of a default exported constant',
         {
           sourceOffsets: [15],
           generatedOffsets: [70],
-          lengths: [6],
+          lengths: [7],
           data: {
             completion: true,
             format: false,
@@ -815,6 +825,7 @@ test('create virtual code w/ MDX layout in case of a default exported constant',
         '',
         'const MDXLayout = "main"',
         '',
+        '',
         '/**',
         ' * @deprecated',
         ' *   Do not use.',
@@ -904,7 +915,7 @@ test('create virtual code w/ MDX layout and matching argument name', () => {
         {
           sourceOffsets: [15],
           generatedOffsets: [726],
-          lengths: [33],
+          lengths: [34],
           data: {
             completion: true,
             format: false,
@@ -934,6 +945,7 @@ test('create virtual code w/ MDX layout and matching argument name', () => {
         ' */',
         'const MDXLayout = function MDXLayout(properties) {}',
         '',
+        '',
         '/**',
         ' * @deprecated',
         ' *   Do not use.',
@@ -1022,9 +1034,9 @@ test('create virtual code w/ MDX layout in case of a default export followed by
       languageId: 'javascriptreact',
       mappings: [
         {
-          sourceOffsets: [15, 39],
-          generatedOffsets: [721, 745],
-          lengths: [23, 26],
+          sourceOffsets: [15],
+          generatedOffsets: [721],
+          lengths: [51],
           data: {
             completion: true,
             format: false,
@@ -1055,6 +1067,7 @@ test('create virtual code w/ MDX layout in case of a default export followed by
         'const MDXLayout = function MDXLayout() {}',
         'export function named() {}',
         '',
+        '',
         '/**',
         ' * @deprecated',
         ' *   Do not use.',
@@ -1145,7 +1158,7 @@ test('create virtual code w/ MDX layout in case of a default export preceded by
         {
           sourceOffsets: [0, 42],
           generatedOffsets: [51, 748],
-          lengths: [26, 23],
+          lengths: [27, 24],
           data: {
             completion: true,
             format: false,
@@ -1176,6 +1189,7 @@ test('create virtual code w/ MDX layout in case of a default export preceded by
         ' */',
         'const MDXLayout = function MDXLayout() {}',
         '',
+        '',
         '/**',
         ' * @deprecated',
         ' *   Do not use.',