apply suggestions from code review

vaadin · Aug 6, 2024 · 7f01d49 · 7f01d49
1 parent 4bcd74a
commit 7f01d49
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 38 deletions.
diff --git a/packages/ts/generator-plugin-signals/src/SignalProcessor.ts b/packages/ts/generator-plugin-signals/src/SignalProcessor.ts
@@ -14,7 +14,6 @@ const HILLA_REACT_SIGNALS = '@vaadin/hilla-react-signals';
 
 const NUMBER_SIGNAL_CHANNEL = '$NUMBER_SIGNAL_CHANNEL$';
 const CONNECT_CLIENT = '$CONNECT_CLIENT$';
-const ENDPOINT_AND_METHOD_NAME = '$ENDPOINT_AND_METHOD_NAME$';
 
 const signalImportPaths = ['com/vaadin/hilla/signals/NumberSignal'];
 
@@ -47,17 +46,13 @@ export default class SignalProcessor {
       ...this.#methods.map((method) =>
         transform<SourceFile>((tsNode) => {
           if (ts.isFunctionDeclaration(tsNode) && tsNode.name?.text === method.name) {
-            const endpointAndMethodName = ts.factory.createStringLiteral(`${this.#service}.${method.name}`);
             const body = template(
               `
 function dummy() {
-  return new ${NUMBER_SIGNAL_CHANNEL}(${ENDPOINT_AND_METHOD_NAME}, ${CONNECT_CLIENT}).signal;
+  return new ${NUMBER_SIGNAL_CHANNEL}('${this.#service}.${method.name}', ${CONNECT_CLIENT}).signal;
 }`,
               (statements) => (statements[0] as FunctionDeclaration).body?.statements,
               [
-                transform((node) =>
-                  ts.isIdentifier(node) && node.text === ENDPOINT_AND_METHOD_NAME ? endpointAndMethodName : node,
-                ),
                 transform((node) =>
                   ts.isIdentifier(node) && node.text === NUMBER_SIGNAL_CHANNEL ? numberSignalChannelId : node,
                 ),
@@ -70,8 +65,7 @@ function dummy() {
               returnType &&
               ts.isTypeReferenceNode(returnType) &&
               'text' in returnType.typeName &&
-              // eslint-disable-next-line @typescript-eslint/no-unsafe-enum-comparison
-              returnType.typeName.escapedText === 'Promise'
+              returnType.typeName.text === 'Promise'
             ) {
               if (returnType.typeArguments && returnType.typeArguments.length > 0) {
                 returnType = returnType.typeArguments[0];
@@ -105,7 +99,7 @@ function dummy() {
     );
   }
 
-  #processSignalImports(signalImports: string[]) {
+  #processSignalImports(signalImports: readonly string[]) {
     const { imports } = this.#dependencyManager;
 
     signalImports.forEach((signalImport) => {

diff --git a/packages/ts/generator-plugin-signals/src/index.ts b/packages/ts/generator-plugin-signals/src/index.ts
@@ -1,5 +1,6 @@
 import Plugin from '@vaadin/hilla-generator-core/Plugin.js';
 import type SharedStorage from '@vaadin/hilla-generator-core/SharedStorage.js';
+import type { OpenAPIV3 } from 'openapi-types';
 import SignalProcessor, { type MethodInfo } from './SignalProcessor.js';
 
 export type PathSignalType = Readonly<{
@@ -10,44 +11,39 @@ export type PathSignalType = Readonly<{
 const SIGNAL_CLASSES = ['#/components/schemas/com.vaadin.hilla.signals.NumberSignal'];
 
 function extractEndpointMethodsWithSignalsAsReturnType(storage: SharedStorage): PathSignalType[] {
-  const pathSignalTypes: PathSignalType[] = [];
-  Object.entries(storage.api.paths).forEach(([path, pathObject]) => {
-    const response200 = pathObject?.post?.responses['200'];
-    if (response200 && !('$ref' in response200)) {
-      // OpenAPIV3.ResponseObject
-      const responseSchema = response200.content?.['application/json'].schema;
-      if (responseSchema && 'anyOf' in responseSchema) {
-        // OpenAPIV3.SchemaObject
-        // eslint-disable-next-line array-callback-return
-        responseSchema.anyOf?.some((c) => {
-          const isSignal = '$ref' in c && c.$ref && SIGNAL_CLASSES.includes(c.$ref);
-          if (isSignal) {
-            pathSignalTypes.push({ path, signalType: c.$ref });
-          }
-        });
-      }
-    }
-  });
-  return pathSignalTypes;
-}
+  return Object.entries(storage.api.paths)
+    .filter(([_, pathObject]) => {
+      const response200 = pathObject?.post?.responses['200'];
+      return response200 && !('$ref' in response200);
+    })
+    .map(([path, pathObject]) => {
+      const response200 = pathObject?.post?.responses['200'];
+      const responseSchema = (response200 as OpenAPIV3.ResponseObject).content?.['application/json']?.schema;
 
-function groupByService(signals: PathSignalType[]): Map<string, MethodInfo[]> {
-  const serviceMap = new Map<string, MethodInfo[]>();
+      return responseSchema && 'anyOf' in responseSchema
+        ? responseSchema.anyOf
+            ?.filter((c) => '$ref' in c && c.$ref && SIGNAL_CLASSES.includes(c.$ref))
+            .map((c: OpenAPIV3.ReferenceObject | OpenAPIV3.SchemaObject) => ({
+              path,
+              signalType: '$ref' in c ? c.$ref : '',
+            }))
+        : [];
+    })
+    .filter((signalArray) => signalArray !== undefined)
+    .reduce<PathSignalType[]>((acc, current) => acc.concat(current), []);
+}
 
-  signals.forEach((signal) => {
+function groupByService(signals: readonly PathSignalType[]): Map<string, MethodInfo[]> {
+  return signals.reduce((serviceMap, signal) => {
     const [_, service, method] = signal.path.split('/');
-
     const serviceMethods = serviceMap.get(service) ?? [];
-
     serviceMethods.push({
       name: method,
       signalType: signal.signalType,
     });
-
     serviceMap.set(service, serviceMethods);
-  });
-
-  return serviceMap;
+    return serviceMap;
+  }, new Map<string, MethodInfo[]>());
 }
 
 export default class SignalsPlugin extends Plugin {

diff --git a/packages/ts/generator-utils/src/dependencies/ImportManager.ts b/packages/ts/generator-utils/src/dependencies/ImportManager.ts
@@ -37,6 +37,17 @@ export class NamedImportManager extends StatementRecordManager<ImportDeclaration
     }
   }
 
+  find(predicate: (path: string, specifier: string) => boolean): [string, string, boolean, Identifier] | undefined {
+    for (const [path, specifiers] of this.#map) {
+      for (const [specifier, { id, isType }] of specifiers) {
+        if (predicate(path, specifier)) {
+          return [path, specifier, isType, id];
+        }
+      }
+    }
+    return undefined;
+  }
+
   override clear(): void {
     this.#map.clear();
   }
@@ -105,6 +116,21 @@ export class NamespaceImportManager extends StatementRecordManager<ImportDeclara
     return id;
   }
 
+  remove(path: string): void {
+    if (this.#map.has(path)) {
+      this.#map.delete(path);
+    }
+  }
+
+  find(predicate: (id: Identifier) => boolean): string | undefined {
+    for (const [path, id] of this.#map) {
+      if (predicate(id)) {
+        return path;
+      }
+    }
+    return undefined;
+  }
+
   override clear(): void {
     this.#map.clear();
   }