Skip to content

Commit

Permalink
fix failing tests after path-to-regexp 6.2.0 update (#549)
Browse files Browse the repository at this point in the history
* fix failing tests after path-to-regexp 6.2.0 update

- since `path-to-regexp` 3.1.0 the way of passing options into the `tokensToFunction()` function has changed (see [#191](pillarjs/path-to-regexp#191)).
- since `path-to-regexp` 4.0.0 the default exports have changed from a module object to individual functions (see [4.0.0](https://github.com/pillarjs/path-to-regexp/releases/tag/v4.0.0)).
- since `path-to-regexp` 5.0.0 the default value for the optional `encode` parameter in the `tokensToFunction()` function has changed (see [5.0.0](https://github.com/pillarjs/path-to-regexp/releases/tag/v5.0.0)).
- since `path-to-regexp` 6.0.0 the `repeat` and `modifier` properties on route regexp keys were removed (see [#207](pillarjs/path-to-regexp#207)).

The internals of the `@vaadin/router` package affected by these changes are updated so that the Vaadin Router public API remains unchanged.

* chore: fix linter errors - declare `xit` global

* fix: do not re-export pathToRegexp in the ES bundle

In the ES module bundle the users may as well just import path-to-regexp themselves.

In the UMD bundle, keep things 'as-is'.
  • Loading branch information
Viktor Lukashov authored Feb 1, 2021
1 parent 9d98980 commit b081aa3
Show file tree
Hide file tree
Showing 8 changed files with 28 additions and 20 deletions.
2 changes: 2 additions & 0 deletions index.polyfilled.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import {Router, Resolver} from './index.js';
import * as pathToRegexp from 'path-to-regexp';
Resolver.pathToRegexp = pathToRegexp;

let isUrlAvailable, urlDocument, urlBase, urlAnchor;

Expand Down
22 changes: 12 additions & 10 deletions src/resolver/generateUrls.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
* LICENSE.txt file in the root directory of this source tree.
*/

import {parse, tokensToFunction} from 'path-to-regexp';
import Resolver from './resolver.js';
import {isString} from '../utils.js';

const {pathToRegexp} = Resolver;
const cache = new Map();

function cacheRoutes(routesByName, route, routes) {
Expand Down Expand Up @@ -49,7 +49,9 @@ function getRoutePath(route) {
return path !== undefined ? path : '';
}

function generateUrls(router, options = {}) {
function generateUrls(router, options = {
encode: encodeURIComponent
}) {
if (!(router instanceof Resolver)) {
throw new TypeError('An instance of Resolver is expected');
}
Expand All @@ -68,8 +70,8 @@ function generateUrls(router, options = {}) {
}
}

let regexp = cache.get(route.fullPath);
if (!regexp) {
let cached = cache.get(route.fullPath);
if (!cached) {
let fullPath = getRoutePath(route);
let rt = route.parent;
while (rt) {
Expand All @@ -79,27 +81,27 @@ function generateUrls(router, options = {}) {
}
rt = rt.parent;
}
const tokens = pathToRegexp.parse(fullPath);
const toPath = pathToRegexp.tokensToFunction(tokens);
const tokens = parse(fullPath);
const keys = Object.create(null);
for (let i = 0; i < tokens.length; i++) {
if (!isString(tokens[i])) {
keys[tokens[i].name] = true;
}
}
regexp = {toPath, keys};
cache.set(fullPath, regexp);
cached = {tokens, keys};
cache.set(fullPath, cached);
route.fullPath = fullPath;
}

let url = regexp.toPath(params, options) || '/';
const toPath = tokensToFunction(cached.tokens, options);
let url = toPath(params) || '/';

if (options.stringifyQueryParams && params) {
const queryParams = {};
const keys = Object.keys(params);
for (let i = 0; i < keys.length; i++) {
const key = keys[i];
if (!regexp.keys[key]) {
if (!cached.keys[key]) {
queryParams[key] = params[key];
}
}
Expand Down
6 changes: 4 additions & 2 deletions src/resolver/matchPath.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,10 @@ function matchPath(routepath, path, exact, parentKeys, parentParams) {
const prop = key.name;
const value = m[i];
if (value !== undefined || !hasOwnProperty.call(params, prop)) {
if (key.repeat) {
params[prop] = value ? value.split(key.delimiter).map(decodeParam) : [];
if (key.modifier === '+' || key.modifier === '*') {
// by default, as of path-to-regexp 6.0.0, the default delimiters
// are `/`, `#` and `?`.
params[prop] = value ? value.split(/[/?#]/).map(decodeParam) : [];
} else {
params[prop] = value ? decodeParam(value) : value;
}
Expand Down
3 changes: 0 additions & 3 deletions src/resolver/resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
* LICENSE.txt file in the root directory of this source tree.
*/

import {pathToRegexp} from 'path-to-regexp';
import matchRoute from './matchRoute.js';
import resolveRoute from './resolveRoute.js';
import {toArray, ensureRoutes, isString, getNotFoundError, notFoundResult} from '../utils.js';
Expand Down Expand Up @@ -249,6 +248,4 @@ class Resolver {
}
}

Resolver.pathToRegexp = pathToRegexp;

export default Resolver;
5 changes: 3 additions & 2 deletions src/router.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {compile} from 'path-to-regexp';
import Resolver from './resolver/resolver.js';
import generateUrls from './resolver/generateUrls.js';
import setNavigationTriggers from './triggers/setNavigationTriggers.js';
Expand Down Expand Up @@ -39,7 +40,7 @@ function createLocation({pathname = '', search = '', hash = '', chain = [], para
params,
redirectFrom,
getUrl: (userParams = {}) => getPathnameForRouter(
Router.pathToRegexp.compile(
compile(
getMatchedPath(routes)
)(Object.assign({}, params, userParams)),
resolver
Expand Down Expand Up @@ -979,7 +980,7 @@ export class Router extends Resolver {
*/
urlForPath(path, params) {
return getPathnameForRouter(
Router.pathToRegexp.compile(path)(params),
compile(path)(params),
this
);
}
Expand Down
1 change: 1 addition & 0 deletions test/.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"afterEach": false,
"fixture": false,
"it": false,
"xit": false,
"expect": false,
"gemini": false,
"sinon": false,
Expand Down
3 changes: 2 additions & 1 deletion test/router/router.spec.html
Original file line number Diff line number Diff line change
Expand Up @@ -796,7 +796,8 @@
await router.ready;
});

it('should invoke pathToRegexp', async() => {
// cannot mock the call to `compile()` from the 'pathToRegexp' package
xit('should invoke pathToRegexp', async() => {
router.setRoutes([
{path: '/a/:id', component: 'x-a'}
]);
Expand Down
6 changes: 4 additions & 2 deletions test/router/url-for.spec.html
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,8 @@
expect(router.urlForName('without-slash')).to.equal('/base/bar');
});

it('should use pathToRegexp', () => {
// cannot mock the call to `parse()` from the 'pathToRegexp' package
xit('should use pathToRegexp', () => {
const parse = sinon.spy(Vaadin.Router.pathToRegexp, 'parse');

try {
Expand Down Expand Up @@ -256,7 +257,8 @@
expect(router.urlForPath('/bar')).to.equal('/base/bar');
});

it('should use pathToRegexp', () => {
// cannot mock the call to `compile()` from the 'pathToRegexp' package
xit('should use pathToRegexp', () => {
const compiledRegExp = sinon.stub().returns('/ok/url');
const compile = sinon.stub(Vaadin.Router.pathToRegexp, 'compile').returns(compiledRegExp);

Expand Down

0 comments on commit b081aa3

Please # to comment.