refactor: do not modify source file on duplicate entries

Author: thetutlage

src/code_transformer/main.ts

@@ -76,29 +76,29 @@ export class CodeTransformer {
     /**
      * Delete the existing middleware if it exists
      */
-    const existingMiddleware = arrayLiteralExpression
+    const existingMiddlewareIndex = arrayLiteralExpression
       .getElements()
       .findIndex((element) => element.getText() === middleware)
 
-    if (existingMiddleware !== -1) {
-      arrayLiteralExpression.removeElement(existingMiddleware)
-    }
-
-    /**
-     * Add the middleware to the top or bottom of the array
-     */
-    if (middlewareEntry.position === 'before') {
-      arrayLiteralExpression.insertElement(0, middleware)
-    } else {
-      arrayLiteralExpression.addElement(middleware)
+    if (existingMiddlewareIndex === -1) {
+      /**
+       * Add the middleware to the top or bottom of the array
+       */
+      if (middlewareEntry.position === 'before') {
+        arrayLiteralExpression.insertElement(0, middleware)
+      } else {
+        arrayLiteralExpression.addElement(middleware)
+      }
     }
   }
 
   /**
    * Add a new middleware to the named middleware of the given file
    */
   #addToNamedMiddleware(file: SourceFile, middlewareEntry: AddMiddlewareEntry) {
-    if (!middlewareEntry.name) throw new Error('Named middleware requires a name.')
+    if (!middlewareEntry.name) {
+      throw new Error('Named middleware requires a name.')
+    }
 
     const callArguments = file
       .getVariableDeclarationOrThrow('middleware')
@@ -118,20 +118,22 @@ export class CodeTransformer {
      * Check if property is already defined. If so, remove it
      */
     const existingProperty = namedMiddlewareObject.getProperty(middlewareEntry.name)
-    if (existingProperty) existingProperty.remove()
-
-    /**
-     * Add the named middleware
-     */
-    const middleware = `${middlewareEntry.name}: () => import('${middlewareEntry.path}')`
-    namedMiddlewareObject!.insertProperty(0, middleware)
+    if (!existingProperty) {
+      /**
+       * Add the named middleware
+       */
+      const middleware = `${middlewareEntry.name}: () => import('${middlewareEntry.path}')`
+      namedMiddlewareObject!.insertProperty(0, middleware)
+    }
   }
 
   /**
    * Write a leading comment
    */
   #addLeadingComment(writer: CodeBlockWriter, comment?: string) {
-    if (!comment) return writer.blankLine()
+    if (!comment) {
+      return writer.blankLine()
+    }
 
     return writer
       .blankLine()
@@ -202,7 +204,7 @@ export class CodeTransformer {
       throw new Error(`The second argument of Env.create is not an object literal.`)
     }
 
-    let firstAdded = false
+    let shouldAddComment = true
 
     /**
      * Add each variable validation
@@ -212,17 +214,32 @@ export class CodeTransformer {
        * Check if the variable is already defined. If so, remove it
        */
       const existingProperty = objectLiteralExpression.getProperty(variable)
-      if (existingProperty) existingProperty.remove()
-
-      objectLiteralExpression.addPropertyAssignment({
-        name: variable,
-        initializer: validation,
-        leadingTrivia: (writer) => {
-          if (firstAdded) return
-          firstAdded = true
-          return this.#addLeadingComment(writer, definition.leadingComment)
-        },
-      })
+
+      /**
+       * Do not add leading comment if one or more properties
+       * already exists
+       */
+      if (existingProperty) {
+        shouldAddComment = false
+      }
+
+      /**
+       * Add property only when the property does not exist
+       */
+      if (!existingProperty) {
+        objectLiteralExpression.addPropertyAssignment({
+          name: variable,
+          initializer: validation,
+          leadingTrivia: (writer) => {
+            if (!shouldAddComment) {
+              return
+            }
+
+            shouldAddComment = false
+            return this.#addLeadingComment(writer, definition.leadingComment)
+          },
+        })
+      }
     }
 
     file.formatText(this.#editorSettings)

tests/__snapshots__/code_transformer.spec.ts.cjs

@@ -294,3 +294,23 @@ export const middleware = router.named({
 })
 "`
 
+exports[`Code transformer | addMiddlewareToStack > do not add duplicate named middleware 1`] = `"import router from '@adonisjs/core/services/router'
+import server from '@adonisjs/core/services/server'
+
+server.errorHandler(() => import('#exceptions/handler'))
+
+server.use([
+  () => import('#middleware/container_bindings_middleware'),
+  () => import('@adonisjs/session/session_middleware'),
+])
+
+router.use([
+  () => import('@adonisjs/core/bodyparser_middleware'),
+  () => import('@adonisjs/shield/shield_middleware'),
+])
+
+export const middleware = router.named({
+  auth: () => import('#foo/bar.js')
+})
+"`
+

tests/code_transformer.spec.ts

@@ -88,7 +88,7 @@ test.group('Code transformer | addMiddlewareToStack', (group) => {
     assert.fileContains('start/kernel.ts', `rand: () => import('@adonisjs/random_middleware')`)
   })
 
-  test('override duplicates when adding router/server middleware', async ({ assert, fs }) => {
+  test('do not add duplicate router/server middleware', async ({ assert, fs }) => {
     const transformer = new CodeTransformer(fs.baseUrl)
 
     await transformer.addMiddlewareToStack('router', [
@@ -112,7 +112,7 @@ test.group('Code transformer | addMiddlewareToStack', (group) => {
     assert.equal(occurrences2, 1)
   })
 
-  test('override duplicates when adding named middelware', async ({ assert, fs }) => {
+  test('do not add duplicate named middleware', async ({ assert, fs }) => {
     const transformer = new CodeTransformer(fs.baseUrl)
 
     await transformer.addMiddlewareToStack('named', [{ name: 'auth', path: '#foo/bar.js' }])
@@ -158,7 +158,7 @@ test.group('Code transformer | defineEnvValidations', (group) => {
     assert.snapshot(file).match()
   })
 
-  test('should replace duplicates', async ({ assert, fs }) => {
+  test('do not add duplicates', async ({ assert, fs }) => {
     const transformer = new CodeTransformer(fs.baseUrl)
 
     await transformer.defineEnvValidations({
@@ -171,7 +171,10 @@ test.group('Code transformer | defineEnvValidations', (group) => {
     const occurrences = (file.match(/NODE_ENV/g) || []).length
 
     assert.equal(occurrences, 1)
-    assert.fileContains('start/env.ts', `NODE_ENV: Env.schema.string.optional()`)
+    assert.fileContains(
+      'start/env.ts',
+      `Env.schema.enum(['development', 'production', 'test'] as const)`
+    )
   })
 })