fix(javascript): remove duplicate echoRequester logic (#650)

Author: shortcuts

clients/algoliasearch-client-javascript/packages/client-common/src/createEchoRequester.ts

@@ -1,26 +1,43 @@
 import type { EchoResponse, EndRequest, Request, Response } from './types';
 
-export type UrlParams = {
-  host: string;
-  algoliaAgent: string;
-  searchParams: EchoResponse['searchParams'];
-};
-
 export type EchoRequesterParams = {
-  getUrlParams: (url: string) => UrlParams;
+  getURL: (url: string) => URL;
   status?: number;
 };
 
+function getUrlParams({
+  host,
+  searchParams: urlSearchParams,
+}: URL): Pick<EchoResponse, 'algoliaAgent' | 'host' | 'searchParams'> {
+  const algoliaAgent = urlSearchParams.get('x-algolia-agent') || '';
+  const searchParams = {};
+
+  for (const [k, v] of urlSearchParams) {
+    if (k === 'x-algolia-agent') {
+      continue;
+    }
+
+    searchParams[k] = v;
+  }
+
+  return {
+    host,
+    algoliaAgent,
+    searchParams:
+      Object.entries(searchParams).length === 0 ? undefined : searchParams,
+  };
+}
+
 // eslint-disable-next-line @typescript-eslint/explicit-function-return-type
 export function createEchoRequester({
-  getUrlParams,
+  getURL,
   status = 200,
 }: EchoRequesterParams) {
   function send(
     { headers, url, connectTimeout, responseTimeout }: EndRequest,
     { data, ...originalRequest }: Request
   ): Promise<Response> {
-    const { host, searchParams, algoliaAgent } = getUrlParams(url);
+    const { host, searchParams, algoliaAgent } = getUrlParams(getURL(url));
     const originalData =
       data && Object.entries(data).length > 0 ? data : undefined;
 
@@ -31,7 +48,7 @@ export function createEchoRequester({
         headers,
         connectTimeout,
         responseTimeout,
-        algoliaAgent: algoliaAgent ? encodeURI(algoliaAgent) : undefined,
+        algoliaAgent: encodeURI(algoliaAgent),
         searchParams,
         data: originalData,
       }),
@@ -42,3 +59,5 @@ export function createEchoRequester({
 
   return { send };
 }
+
+export type EchoRequester = ReturnType<typeof createEchoRequester>;

clients/algoliasearch-client-javascript/packages/client-common/src/types/Requester.ts

@@ -47,6 +47,6 @@ export type EchoResponse = Request & {
   host: string;
   headers: Headers;
   responseTimeout: number;
+  algoliaAgent: string;
   searchParams?: Record<string, string>;
-  algoliaAgent?: string;
 };

clients/algoliasearch-client-javascript/packages/requester-browser-xhr/src/echoRequester.ts

@@ -1,28 +1,6 @@
 import { createEchoRequester } from '@experimental-api-clients-automation/client-common';
-import type { UrlParams } from '@experimental-api-clients-automation/client-common';
+import type { EchoRequester } from '@experimental-api-clients-automation/client-common';
 
-function getUrlParams(url: string): UrlParams {
-  const { host, searchParams: urlSearchParams } = new URL(url);
-  const algoliaAgent = urlSearchParams.get('x-algolia-agent') || '';
-  const searchParams = {};
-
-  for (const [k, v] of urlSearchParams) {
-    if (k === 'x-algolia-agent') {
-      continue;
-    }
-
-    searchParams[k] = v;
-  }
-
-  return {
-    host,
-    algoliaAgent,
-    searchParams:
-      Object.entries(searchParams).length === 0 ? undefined : searchParams,
-  };
-}
-
-// eslint-disable-next-line @typescript-eslint/explicit-function-return-type
-export function echoRequester(status: number = 200) {
-  return createEchoRequester({ getUrlParams, status });
+export function echoRequester(status: number = 200): EchoRequester {
+  return createEchoRequester({ getURL: (url: string) => new URL(url), status });
 }

clients/algoliasearch-client-javascript/packages/requester-node-http/src/echoRequester.ts

@@ -1,30 +1,8 @@
 import { URL } from 'url';
 
 import { createEchoRequester } from '@experimental-api-clients-automation/client-common';
-import type { UrlParams } from '@experimental-api-clients-automation/client-common';
+import type { EchoRequester } from '@experimental-api-clients-automation/client-common';
 
-function getUrlParams(url: string): UrlParams {
-  const { host, searchParams: urlSearchParams } = new URL(url);
-  const algoliaAgent = urlSearchParams.get('x-algolia-agent') || '';
-  const searchParams = {};
-
-  for (const [k, v] of urlSearchParams) {
-    if (k === 'x-algolia-agent') {
-      continue;
-    }
-
-    searchParams[k] = v;
-  }
-
-  return {
-    host,
-    algoliaAgent,
-    searchParams:
-      Object.entries(searchParams).length === 0 ? undefined : searchParams,
-  };
-}
-
-// eslint-disable-next-line @typescript-eslint/explicit-function-return-type
-export function echoRequester(status: number = 200) {
-  return createEchoRequester({ getUrlParams, status });
+export function echoRequester(status: number = 200): EchoRequester {
+  return createEchoRequester({ getURL: (url: string) => new URL(url), status });
 }

clients/algoliasearch-client-javascript/rollup.config.js

@@ -14,7 +14,8 @@ const client = process.env.CLIENT?.replace(
   ''
 );
 const UTILS = ['client-common', 'requester-browser-xhr', 'requester-node-http'];
-
+const BROWSER_FORMATS = ['umd-browser', 'esm-browser', 'cjs-browser'];
+const NODE_FORMATS = ['cjs-node', 'esm-node'];
 const CLIENT_ALL = 'all';
 const CLIENT_UTILS = 'utils';
 
@@ -61,9 +62,9 @@ function getAvailableClients() {
 function getUtilConfigs() {
   const commonOptions = {
     input: 'index.ts',
-    formats: ['cjs-node', 'esm-node'],
+    formats: NODE_FORMATS,
     external: [],
-    dependencies: [],
+    dependencies: ['@experimental-api-clients-automation/client-common'],
   };
 
   return [
@@ -73,6 +74,7 @@ function getUtilConfigs() {
       output: 'client-common',
       package: 'client-common',
       name: '@experimental-api-clients-automation/client-common',
+      dependencies: [],
     },
     // Browser requester
     {
@@ -81,7 +83,6 @@ function getUtilConfigs() {
       package: 'requester-browser-xhr',
       name: '@experimental-api-clients-automation/requester-browser-xhr',
       external: ['dom'],
-      dependencies: ['@experimental-api-clients-automation/client-common'],
     },
     // Node requester
     {
@@ -90,7 +91,6 @@ function getUtilConfigs() {
       package: 'requester-node-http',
       name: '@experimental-api-clients-automation/requester-node-http',
       external: ['https', 'http', 'url'],
-      dependencies: ['@experimental-api-clients-automation/client-common'],
     },
   ];
 }
@@ -142,14 +142,12 @@ function getPackageConfigs() {
         : ['@experimental-api-clients-automation/client-common'],
       external: [],
     };
-    const browserFormats = ['umd-browser', 'esm-browser', 'cjs-browser'];
-    const nodeFormats = ['cjs-node', 'esm-node'];
 
     return [
       {
         ...commonConfig,
         input: 'builds/browser.ts',
-        formats: browserFormats,
+        formats: BROWSER_FORMATS,
         external: ['dom'],
         dependencies: [
           ...commonConfig.dependencies,
@@ -166,7 +164,7 @@ function getPackageConfigs() {
           ...commonConfig.dependencies,
           '@experimental-api-clients-automation/requester-node-http',
         ],
-        formats: nodeFormats,
+        formats: NODE_FORMATS,
       },
     ];
   });

scripts/ci/githubActions/setRunVariables.ts

@@ -1,4 +1,5 @@
 /* eslint-disable no-console */
+import { CLIENTS_JS_UTILS } from '../../common';
 import { getLanguageFolder } from '../../config';
 
 import { isBaseChanged } from './utils';
@@ -53,11 +54,9 @@ export const DEPENDENCIES = {
   JS_COMMON_TESTS_CHANGED: [
     `${JS_CLIENT_FOLDER}/packages/client-common/src/__tests__`,
   ],
-  JAVASCRIPT_UTILS_CHANGED: [
-    `${JS_CLIENT_FOLDER}/packages/client-common`,
-    `${JS_CLIENT_FOLDER}/packages/requester-browser-xhr`,
-    `${JS_CLIENT_FOLDER}/packages/requester-node-http`,
-  ],
+  JAVASCRIPT_UTILS_CHANGED: CLIENTS_JS_UTILS.map(
+    (clientName) => `${JS_CLIENT_FOLDER}/packages/${clientName}`
+  ),
   JAVASCRIPT_CLIENT_CHANGED: [
     ...CLIENTS_COMMON_FILES,
     JS_CLIENT_FOLDER,

templates/javascript/tests/client/method.mustache

@@ -1 +1 @@
-{{object}}{{#path}}.{{.}}{{/path}}({{{parameters}}});
+{{object}}{{#path}}.{{.}}{{/path}}({{{parameters}}})

templates/javascript/tests/client/step.mustache

@@ -5,5 +5,5 @@
   const result = {{> client/variable}}
 {{/isVariable}}
 {{#isMethod}}
-  const result = await {{> client/method}}
+  const result = await ({{> client/method}}) as unknown as EchoResponse;
 {{/isMethod}}

templates/javascript/tests/client/suite.mustache

@@ -1,8 +1,8 @@
-/* eslint-disable @typescript-eslint/no-unused-vars */
-/* eslint-disable require-await */
+/* eslint-disable @typescript-eslint/no-unused-vars, require-await */
 // @ts-nocheck Failing tests will have type errors, but we cannot suppress them even with @ts-expect-error because it doesn't work for a block of lines.
 import { {{client}}, {{#lambda.titlecase}}{{client}}{{/lambda.titlecase}} } from '{{{import}}}';
 import { echoRequester } from '@experimental-api-clients-automation/requester-node-http';
+import type { EchoResponse } from '@experimental-api-clients-automation/requester-node-http';
 
 const appId = 'test-app-id';
 const apiKey = 'test-api-key';
@@ -25,7 +25,7 @@ describe('{{testType}}', () => {
           {{> client/step}}
           throw new Error('test is expected to throw error');
         } catch(e) {
-          expect(e.message).toMatch("{{{expectedError}}}");
+          expect((e as Error).message).toMatch("{{{expectedError}}}");
         }
       {{/isError}}
       {{^isError}}