From 01cfb1dce43b6f9c4a94a9e5a8eb05736b5c2899 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 7 Mar 2021 14:04:49 +0100 Subject: [PATCH] core-api: added tests for SubRouteRef + improve path validation Signed-off-by: Patrik Oldsberg --- .../core-api/src/routing/SubRouteRef.test.ts | 134 ++++++++++++++++++ packages/core-api/src/routing/SubRouteRef.ts | 20 ++- 2 files changed, 151 insertions(+), 3 deletions(-) create mode 100644 packages/core-api/src/routing/SubRouteRef.test.ts diff --git a/packages/core-api/src/routing/SubRouteRef.test.ts b/packages/core-api/src/routing/SubRouteRef.test.ts new file mode 100644 index 0000000000..1c1a3c1b21 --- /dev/null +++ b/packages/core-api/src/routing/SubRouteRef.test.ts @@ -0,0 +1,134 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { AnyParams, SubRouteRef } from './types'; +import { createSubRouteRef, isSubRouteRef } from './SubRouteRef'; +import { createRouteRef, isRouteRef } from './RouteRef'; +import { isExternalRouteRef } from './ExternalRouteRef'; + +const parent = createRouteRef({ id: 'parent' }); +const parentX = createRouteRef({ id: 'parent-x', params: ['x'] }); + +describe('SubRouteRef', () => { + it('should be created', () => { + const routeRef: SubRouteRef = createSubRouteRef({ + parent, + id: 'my-route-ref', + path: '/foo', + }); + expect(routeRef.path).toBe('/foo'); + expect(routeRef.parent).toBe(parent); + expect(routeRef.params).toEqual([]); + expect(String(routeRef)).toBe('routeRef{type=sub,id=my-route-ref}'); + expect(isRouteRef(routeRef)).toBe(false); + expect(isSubRouteRef(routeRef)).toBe(true); + expect(isExternalRouteRef(routeRef)).toBe(false); + + expect(isRouteRef({} as SubRouteRef)).toBe(false); + }); + + it('should be created with params', () => { + const routeRef: SubRouteRef<{ bar: string }> = createSubRouteRef({ + parent, + id: 'my-other-route-ref', + path: '/foo/:bar', + }); + expect(routeRef.path).toBe('/foo/:bar'); + expect(routeRef.parent).toBe(parent); + expect(routeRef.params).toEqual(['bar']); + }); + + it('should be created with merged params', () => { + const routeRef: SubRouteRef<{ + x: string; + y: string; + z: string; + }> = createSubRouteRef({ + parent: parentX, + id: 'my-other-route-ref', + path: '/foo/:y/:z', + }); + expect(routeRef.path).toBe('/foo/:y/:z'); + expect(routeRef.parent).toBe(parentX); + expect(routeRef.params).toEqual(['x', 'y', 'z']); + }); + + it('should be created with params from parent', () => { + const routeRef: SubRouteRef<{ x: string }> = createSubRouteRef({ + parent: parentX, + id: 'my-other-route-ref', + path: '/foo/bar', + }); + expect(routeRef.path).toBe('/foo/bar'); + expect(routeRef.parent).toBe(parentX); + expect(routeRef.params).toEqual(['x']); + }); + + it.each([ + ['foo', "SubRouteRef path must start with '/', got 'foo'"], + [':foo', "SubRouteRef path must start with '/', got ':foo'"], + ['', "SubRouteRef path must start with '/', got ''"], + ['/', "SubRouteRef path must not end with '/', got '/'"], + ['/foo/', "SubRouteRef path must not end with '/', got '/foo/'"], + ['/foo/:x', 'SubRouteRef may not have params that overlap with its parent'], + ['/:/foo', "SubRouteRef path has invalid param, got ''"], + ['/:inva:lid/foo', "SubRouteRef path has invalid param, got 'inva:lid'"], + ['/:inva=lid/foo', "SubRouteRef path has invalid param, got 'inva=lid'"], + ])('should throw if path is invalid, %s', (path, message) => { + expect(() => + createSubRouteRef({ path, parent: parentX, id: path }), + ).toThrow(message); + }); + + it('should properly infer and parse path parameters', () => { + function validateType(_ref: SubRouteRef) {} + + const _1 = createSubRouteRef({ id: '1', parent, path: '/foo/bar' }); + // @ts-expect-error + validateType<{ x: string }>(_1); + validateType(_1); + + const _2 = createSubRouteRef({ id: '2', parent, path: '/foo/:x/:y' }); + // @ts-expect-error + validateType(_2); + // @ts-expect-error + validateType<{ x: string; z: string }>(_2); + // @ts-expect-error + validateType<{ y: string }>(_2); + // TODO(Rugvip): Ideally this would fail as well, but settle for validating it at runtime instead + validateType<{ x: string; y: string; z: string }>(_2); + validateType<{ x: string; y: string }>(_2); + + const _3 = createSubRouteRef({ id: '3', parent: parentX, path: '/foo' }); + // @ts-expect-error + validateType(_3); + // @ts-expect-error + validateType<{ y: string }>(_3); + validateType<{ x: string }>(_3); + + const _4 = createSubRouteRef({ id: '4', parent: parentX, path: '/foo/:y' }); + // @ts-expect-error + validateType(_4); + // @ts-expect-error + validateType<{ x: string; z: string }>(_4); + // @ts-expect-error + validateType<{ y: string }>(_4); + validateType<{ x: string; y: string }>(_4); + + // To avoid complains about missing expectations and unused vars + expect([_1, _2, _3, _4].join('')).toEqual(expect.any(String)); + }); +}); diff --git a/packages/core-api/src/routing/SubRouteRef.ts b/packages/core-api/src/routing/SubRouteRef.ts index 8ff23c0c5e..7ddfc89c80 100644 --- a/packages/core-api/src/routing/SubRouteRef.ts +++ b/packages/core-api/src/routing/SubRouteRef.ts @@ -24,6 +24,9 @@ import { SubRouteRef, } from './types'; +// Should match the pattern in react-router +const PARAM_PATTERN = /^\w+$/; + export class SubRouteRefImpl implements SubRouteRef { readonly [routeRefType] = 'sub'; @@ -79,16 +82,27 @@ export function createSubRouteRef< type Params = PathParams; // Collect runtime parameters from the path, e.g. ['bar', 'baz'] from '/foo/:bar/:baz' - const pathParams = path.split(/:([^/]+)/).filter((_, i) => i % 2 === 1); + const pathParams = path + .split('/') + .filter(p => p.startsWith(':')) + .map(p => p.substring(1)); const params = [...parent.params, ...pathParams]; if (parent.params.some(p => pathParams.includes(p as string))) { throw new Error( - 'SubRouteRef may not have params that overlap with its parent params', + 'SubRouteRef may not have params that overlap with its parent', ); } if (!path.startsWith('/')) { - throw new Error(`SubRouteRef path sub starts with '/', got '${path}'`); + throw new Error(`SubRouteRef path must start with '/', got '${path}'`); + } + if (path.endsWith('/')) { + throw new Error(`SubRouteRef path must not end with '/', got '${path}'`); + } + for (const param of pathParams) { + if (!PARAM_PATTERN.test(param)) { + throw new Error(`SubRouteRef path has invalid param, got '${param}'`); + } } // We ensure that the type of the return type is sane here