Merge ae1be66 into aaecd79

README.md

@@ -48,6 +48,14 @@ root scope (e.g. window/self for browsers or global for node).
 
 **YES**
 
+#### Using with hermes engine
+
+Make sure to set `useLodashTemplate` config option to `false` to avoid running into this https://github.com/facebook/hermes/issues/222
+hermes issue. See more details in #117.
+
+We do plan to make the new url parameter renderer the default in the next major version but for now we are
+introducing it as opt-in to see how it behaves in the wild.
+
 #### <a name="signal-rn"></a>Using `signal` with react-native
 
 Use [abortcontroller-polyfill](https://github.com/mo/abortcontroller-polyfill) until https://github.com/facebook/react-native/issues/18115 is resolved in react-native core. The polyfill does not actually close the connection, but instead ensures the fetch rejects the promise with `AbortError`. To use the polyfill add the following to the top of your app entrypoint:
@@ -134,6 +142,7 @@ endpoints. It's still possible to use Resources without a router(see [Resource a
 -   `withCredentials` _(bool)_: Allow request backend to send cookies/authentication headers, useful when using same API for server-side rendering.
 -   `allowAttachments` _(bool)_: Allow POST like methods to send attachments.
 -   `signal`: _(AbortSignal)_: Pass in an [AbortSignal](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal) object to abort the request when desired. **Only supported via request config.** Default: [null]. For react-native a [polyfill](#signal-rn) is needed.
+-   `useLodashTemplate` _(bool)_: Set to false to use our own url parameter replacement logic instead of using `lodash.template` based one. Should be set to false when using react-native hermes engine to work around this [issue in hermes](https://github.com/facebook/hermes/issues/222). Default: `true`.
 
 ## Error handling
 

package.json

@@ -91,7 +91,6 @@
     "express": "^4.17.1",
     "form-data": "^4.0.0",
     "fs-extra": "*",
-    "isparta": "*",
     "jest": "^28.1.3",
     "jest-cli": "^28.1.3",
     "jest-extended": "^3.0.2",
@@ -111,10 +110,13 @@
     "tsdx": "^0.14.1",
     "tslib": "^2.4.0",
     "typescript": "^4.7.4",
-    "uuid": "^8.3.2",
-    "watch": "*"
+    "uuid": "^8.3.2"
   },
   "engines": {
     "node": ">=10"
+  },
+  "resolutions": {
+    "node-notifier": ">=10.0.0",
+    "jest-environment-jsdom": ">=28.1.3"
   }
 }

packages/core/package.json

@@ -40,6 +40,7 @@
   },
   "dependencies": {
     "@tg-resources/is": "^3.2.0",
+    "@tg-resources/route-template": "^3.2.0",
     "cookie": ">=0.3.1"
   },
   "scripts": {

packages/core/src/constants.ts

@@ -22,6 +22,8 @@ const DEFAULTS: ConfigType = {
     allowAttachments: false,
 
     signal: null,
+
+    useLodashTemplate: true,
 };
 
 export default DEFAULTS;

packages/core/src/resource.ts

@@ -1,5 +1,5 @@
 import { hasValue, isFunction, isObject, isStatusCode } from '@tg-resources/is';
-import lodashTemplate from 'lodash.template';
+import { routeTemplate } from '@tg-resources/route-template';
 
 import DEFAULTS from './constants';
 import {
@@ -32,15 +32,15 @@ export abstract class Resource extends Route implements ResourceInterface {
      */
     public constructor(apiEndpoint: string, config: RouteConfig = null) {
         super(config);
-        const interpolate = /\$?{([\s\S]+?)}/g;
 
         this._apiEndpoint = apiEndpoint;
-        this._routeTemplate = lodashTemplate(apiEndpoint, { interpolate });
+
+        this._routeTemplate = routeTemplate(apiEndpoint);
     }
 
     private readonly _apiEndpoint: string;
 
-    private readonly _routeTemplate: ReturnType<typeof lodashTemplate>;
+    private readonly _routeTemplate: ReturnType<typeof routeTemplate>;
 
     public get apiEndpoint() {
         return this._apiEndpoint;
@@ -203,15 +203,19 @@ export abstract class Resource extends Route implements ResourceInterface {
         urlParams: Params | null = null,
         requestConfig: RequestConfig = null
     ): string {
-        let thePath = this.apiEndpoint;
         const config = this.config(requestConfig);
 
-        // istanbul ignore next: Tested in package that implement Resource
+        // istanbul ignore next: Tested in package that implements Resource
         if (isObject(urlParams)) {
-            thePath = this._routeTemplate(urlParams);
+            this._routeTemplate.configure(
+                config.apiRoot,
+                config.useLodashTemplate
+            );
+
+            return this._routeTemplate(urlParams);
         }
 
-        return `${config.apiRoot}${thePath}`;
+        return `${config.apiRoot}${this.apiEndpoint}`;
     }
 
     /* Internal API */

packages/core/src/types.ts

@@ -102,6 +102,17 @@ export interface ConfigType {
      */
     signal: Optional<AbortSignal>;
 
+    /**
+     * Should tg-resources use lodash.template or a built-in
+     *  replacement engine for route kwargs. Defaults to true.
+     *
+     * When on react-native with Hermes runtime set it to `false` as it
+     *  doesn't support `with` statement (ref: https://github.com/thorgate/tg-resources/issues/117).
+     *
+     * This may become the default in the next major version however currently it is opt-in.
+     */
+    useLodashTemplate: boolean;
+
     // allow Index Signature
     [key: string]: any;
 }

packages/route-template/src/builtin.ts

@@ -0,0 +1,27 @@
+import { hasValue, isObject } from '@tg-resources/is';
+
+// Needle defaults to {value} or ${value}
+const needle = /\$?{([\s\S]+?)}/g;
+
+function compileURL(template: string) {
+    const render = (params: Record<string, unknown>) => {
+        return template.replace(needle, (_, key: any) => {
+            let value: Record<string, unknown> | unknown = params;
+            let itemKey = key;
+
+            while (itemKey.includes('.')) {
+                const curKey: string = itemKey.split('.')[0];
+                value = isObject(value) ? value?.[curKey] : undefined;
+
+                itemKey = itemKey.split('.').slice(1).join('.');
+            }
+
+            const finalValue = isObject(value) ? value[itemKey] : '';
+            return hasValue(finalValue) ? `${finalValue}` : '';
+        });
+    };
+
+    return render;
+}
+
+export default compileURL;

packages/route-template/src/routeTemplate.ts

@@ -1,11 +1,13 @@
 import lodashTemplate from 'lodash.template';
+import compileURL from './builtin';
 
 import { RouteTemplate, PrepareKwargs } from './types';
 import { cleanRoot, cleanRoute } from './utils';
 
 export function routeTemplate<TKwargs = void>(
     route: string
 ): RouteTemplate<TKwargs>;
+
 export function routeTemplate<PK extends PrepareKwargs<any>>(
     route: string,
     prepareKwargs: PK
@@ -18,17 +20,21 @@ export function routeTemplate(
     // Interpolate defaults to {value} or ${value}
     const interpolate = /\$?{([\s\S]+?)}/g;
 
-    const compiled = lodashTemplate(routePath, { interpolate });
+    let replacer: (params: any) => string;
 
     let currentRoot = '';
-    function configure(apiRoot: string) {
+    function configure(apiRoot: string, lodash = true) {
         currentRoot = cleanRoot(apiRoot);
+
+        replacer = lodash
+            ? lodashTemplate(routePath, { interpolate })
+            : compileURL(routePath);
     }
 
     function renderTemplate(params?: Record<string, unknown>) {
-        const kwargs = prepareKwargs ? prepareKwargs(params) : null;
+        const kwargs = prepareKwargs ? prepareKwargs(params) : params;
 
-        const renderedPath = compiled(kwargs);
+        const renderedPath = replacer(kwargs);
 
         return `${currentRoot}/${cleanRoute(renderedPath)}`;
     }

packages/route-template/src/types.ts

@@ -9,7 +9,7 @@ export type IfPrepareKwargsProvided<
 > = PK extends (...args: any[]) => any ? True : False;
 
 export interface BaseRouteTemplate {
-    configure: (apiRoot: string) => void;
+    configure: (apiRoot: string, lodash?: boolean) => void;
 
     routePath: string;
 }

packages/route-template/test/template.test.ts

@@ -3,32 +3,79 @@ import { routeTemplate, withKwargs } from '../src';
 describe('template format works :: auto slash', () => {
     const apiRoot = '/api';
 
-    test('no-parameters', () => {
+    test.each([true, false])(`[lodash=%s] no-parameters`, (lodash: boolean) => {
         const render = routeTemplate('test/path');
-        render.configure(apiRoot);
+        render.configure(apiRoot, lodash);
 
         expect(render()).toBe('/api/test/path');
     });
 
-    test('with-parameters :: template', () => {
-        const render = routeTemplate(
-            'test/${id}',
-            withKwargs<{ id: number }>()
-        );
-        render.configure(apiRoot);
+    test.each([true, false])(
+        '[lodash=%s] with-parameters :: template',
+        (lodash: boolean) => {
+            const render = routeTemplate(
+                'test/${id}',
+                withKwargs<{ id: number }>()
+            );
+            render.configure(apiRoot, lodash);
 
-        expect(render({ id: 1 })).toBe('/api/test/1');
-    });
+            expect(render({ id: 1 })).toBe('/api/test/1');
+        }
+    );
 
-    test('with-parameters :: mustache', () => {
-        const render = routeTemplate(
-            'test/{id}',
-            (kwargs: { id: number }) => kwargs
-        );
-        render.configure(apiRoot);
+    test.each([true, false])(
+        '[lodash=%s] with-parameters :: mustache',
+        (lodash: boolean) => {
+            const render = routeTemplate(
+                'test/{id}',
+                (kwargs: { id: number }) => kwargs
+            );
+            render.configure(apiRoot, lodash);
 
-        expect(render({ id: 1 })).toBe('/api/test/1');
-    });
+            expect(render({ id: 1 })).toBe('/api/test/1');
+        }
+    );
+
+    test.each([true, false])(
+        '[lodash=%s] nested parameters :: template',
+        (lodash: boolean) => {
+            const render = routeTemplate(
+                'test/${foo.bar.baz}',
+                (kwargs: { foo: { bar: { baz: number } } }) => kwargs
+            );
+            render.configure(apiRoot, lodash);
+
+            expect(render({ foo: { bar: { baz: 1 } } })).toBe('/api/test/1');
+        }
+    );
+
+    test.each([true, false])(
+        '[lodash=%s] nested parameters :: mustache',
+        (lodash: boolean) => {
+            const render = routeTemplate(
+                'test/{foo.bar.baz}',
+                (kwargs: { foo: { bar: { baz: number } } }) => kwargs
+            );
+            render.configure(apiRoot, lodash);
+
+            expect(render({ foo: { bar: { baz: 1 } } })).toBe('/api/test/1');
+        }
+    );
+
+    test.each([true, false])(
+        '[lodash=%s] nested parameters :: missing values',
+        (lodash: boolean) => {
+            const render = routeTemplate(
+                'test/${foo.bar.baz}/${foo.other}/asdasd',
+                (kwargs: { foo: { bar: { baz: number } } }) => kwargs
+            );
+            render.configure(apiRoot, lodash);
+
+            expect(render({ foo: { bar: { baz: 1 } } })).toBe(
+                '/api/test/1//asdasd'
+            );
+        }
+    );
 });
 
 describe('template format works :: fix slash', () => {

packages/route-template/test/utils.test.ts

@@ -0,0 +1,56 @@
+import { isRouteTemplate, routeTemplate } from '../src';
+import { cleanRoot, cleanRoute } from '../src/utils';
+
+describe('cleanRoot', () => {
+    test('cleanRoot works', () => {
+        expect(cleanRoot('/')).toBe('');
+        expect(cleanRoot('/test')).toBe('/test');
+        expect(cleanRoot('/test/')).toBe('/test');
+        expect(cleanRoot('/test/path')).toBe('/test/path');
+        expect(cleanRoot('/test/path/')).toBe('/test/path');
+    });
+});
+
+describe('cleanRoute', () => {
+    test('cleanRoute works', () => {
+        expect(cleanRoute('/')).toBe('');
+        expect(cleanRoute('/test')).toBe('test');
+        expect(cleanRoute('/test/')).toBe('test/');
+        expect(cleanRoute('/test/path')).toBe('test/path');
+        expect(cleanRoute('/test/path/')).toBe('test/path/');
+    });
+});
+
+describe('isRouteTemplate', () => {
+    test('happy path', () => {
+        const method = routeTemplate('/test/path');
+
+        expect(isRouteTemplate(method)).toBe(true);
+    });
+
+    test('not function', () => {
+        const method = 'not a function';
+
+        expect(isRouteTemplate(method)).toBe(false);
+    });
+
+    test('missing routePath', () => {
+        const method = () => {};
+
+        expect(isRouteTemplate(method)).toBe(false);
+    });
+
+    test('routepath is not a string', () => {
+        const method = routeTemplate('/test/path');
+        (method as any).routePath = 1;
+
+        expect(isRouteTemplate(method)).toBe(false);
+    });
+
+    test('missing configure', () => {
+        const method = routeTemplate('/test/path');
+        delete (method as any).configure;
+
+        expect(isRouteTemplate(method)).toBe(false);
+    });
+});

packages/test-server/src/test-server.ts

@@ -204,10 +204,6 @@ function configureServer(logger = false) {
     });
 
     app.post('/attachments', upload.any(), (req, res) => {
-        console.log('req.headers', req.headers);
-        console.log('req.files', req.files);
-        console.log('req.body', req.body);
-
         const files = req.files as Express.Multer.File[];
         const fields = req.body;