Skip to content

Commit

Permalink
fix: 🐛 fix Express JS' path parameter pattern ':id(\\d+)' not being r…
Browse files Browse the repository at this point in the history 
…ecognized
  • Loading branch information
@simplymichael
simplymichael committed Aug 19, 2024
1 parent f13a2e5 commit ee80f54
Show file tree
Hide file tree
Showing 2 changed files with 190 additions and 2 deletions.
70 changes: 68 additions & 2 deletions lib/create-router.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,20 @@ const path = require("path");
const laravelToExpress = require("./laravel-to-express");
const uriWithParams = require("./uri-with-params");

// match a pure express route param regex such as
// :id(\\d+), in e.g., /foo/:id(\\d+)
// which was not supported by node-laravel-router's router.
// Previously to support such regex pattern,
// we'd have to use either
// - the `patterns` option { uri: "foo", patterns: { id: /^\d+$/} } OR
// - :id([0-9]), e.g: /foo/:id([0-9])
//
// We'll use it to split or match the uri to obtain its parameters.
// Wrapping the delimiter regex in parentheses ensures that when we split
// the delimiter also appears as part of the returned array.
const URI_PARAMS_REGEX = /((:\w+|{\w+})(\([\\]+[^()]*\)))/g;
const BACKSLASH_REGEX = /\\/g;
const PARENS_BACKSLASH_REGEX = /(\([\\]+[^()]*\))/g; // /\(([\\]+)\)/g;
const defaultGroupOptions = {
prefix: "/",
middleware: [],
Expand Down Expand Up @@ -52,6 +65,29 @@ function isExpressApp(app) {
);
}

function formatUriAndPatterns(uri) {
let patterns;
const uriParams = uri.match(URI_PARAMS_REGEX);

uri = uri.replace(PARENS_BACKSLASH_REGEX, "").replace(BACKSLASH_REGEX, "/");

if(uriParams?.length) {
patterns = {};
uri = uri.replace(/:(\w+\??)/g, "{$1}"); // replace e.g., :id with {id} and :name? with {name?}

for(const param of uriParams) {
let [key, value] = param.split("(");

key = key.replace(/[:{}]+/g, "");
value = value.substring(0, value.indexOf(")")).replace(/[\\]+/g, "\\");

patterns[key] = new RegExp(`^${value}$`);
}
}

return { uri, patterns };
}

/**
* Create and return Router.
*
Expand Down Expand Up @@ -190,9 +226,20 @@ module.exports = function createRouter(...args) {
const routeOptions = Object.assign({}, defaultRouteOptions, options);

routeOptions.method = routeOptions.method.toLowerCase();


//const uri = path.join.apply(null, this.uris.concat(`/${routeOptions.uri}`));
const uri = path.join.apply(null, this.uris.concat(`${routeOptions.uri}`)).replace(BACKSLASH_REGEX, "/");
let uri = path.join.apply(null, this.uris.concat(`${routeOptions.uri}`));

const { uri: url, patterns: formattedPatterns } = formatUriAndPatterns(uri);

uri = url;

if(formattedPatterns) {
this.patterns = this.patterns.concat(formattedPatterns);
//routeOptions.patterns = { ...routeOptions.patterns, ...formattedPatterns };
}

const middleware = this.middlewares.concat(routeOptions.middleware);
const name = this.names.concat(routeOptions.name).join("");
const patterns = Object.assign.apply(null, [{}].concat(this.patterns, routeOptions.patterns));
Expand Down Expand Up @@ -252,8 +299,27 @@ module.exports = function createRouter(...args) {
const groupOptions = Object.assign({}, defaultGroupOptions, options);
const router = new this.constructor(this);

let parsedPatterns = {};

//router.uris = this.uris.concat(`/${groupOptions.prefix}`);
router.uris = this.uris.concat(`${groupOptions.prefix}`).map(uri => uri.replace(BACKSLASH_REGEX, "/"));
router.uris = this.uris.concat(`${groupOptions.prefix}`).map(uri => {
//uri.replace(BACKSLASH_REGEX, "/")

const { uri: url, patterns: formattedPatterns } = formatUriAndPatterns(uri);

uri = url;

if(formattedPatterns) {
parsedPatterns = { ...parsedPatterns, ...formattedPatterns };
}

return uri;
});

if(Object.keys(parsedPatterns).length > 0) {
this.patterns = this.patterns.concat(parsedPatterns);
}

router.middlewares = this.middlewares.concat(groupOptions.middleware);
router.names = this.names.concat(groupOptions.namespace);
router.patterns = this.patterns.concat(groupOptions.patterns);
Expand Down
122 changes: 122 additions & 0 deletions test/create-router.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,67 @@ describe("createRouter(app:express[, mapActionToHandler:function]):Router", func
}
]
},
{
description: "\"options\" is a uri string that has pure express regex constraint",
options: "/foo/bar/{id}(\\d+)/:name([A-Z-a-z]+)",
action(req, res) {
res.send(`Welcome ${req.params.name ?? "Michael"}. Your ID is ${req.params.id}.`);
},
expected: [
{
url: "/foo/bar/123/James",
method: "get",
status: 200,
response: "Welcome James. Your ID is 123."
},
{
url: "/foo/bar/123/456",
method: "get",
status: 404
},
{
url: "/foo/bar/thing/James",
method: "get",
status: 404
},
{
url: "/foo/bar/James/123",
method: "get",
status: 404
}
]
},
{
description: "\"options\" is a uri string that has optional express regex constraint",
options: "/foo/bar/{id}(\\d+)/:name?",
action(req, res) {
res.send(`Welcome ${req.params.name ?? "Michael"}. Your ID is ${req.params.id}.`);
},
expected: [
{
url: "/foo/bar/123/James",
method: "get",
status: 200,
response: "Welcome James. Your ID is 123."
},
{
url: "/foo/bar/154",
method: "get",
status: 200,
response: "Welcome Michael. Your ID is 154."
},
{
url: "/foo/bar/thing/James",
method: "get",
status: 404
},
{
url: "/foo/bar/James/123",
method: "get",
status: 404
}
]
},
];

tests.forEach(test => {
Expand Down Expand Up @@ -809,6 +870,67 @@ describe("createRouter(app:express[, mapActionToHandler:function]):Router", func
}
]
},
{
description: "\"options\" is a uri string that has pure express regex constraint",
options: "/foo/bar/{id}(\\d+)/:name([A-Z-a-z]+)",
action(req, res) {
res.send(`Welcome ${req.params.name ?? "Michael"}. Your ID is ${req.params.id}.`);
},
expected: [
{
url: "/foo/bar/123/James",
method: "get",
status: 200,
response: "Welcome James. Your ID is 123."
},
{
url: "/foo/bar/123/456",
method: "get",
status: 404
},
{
url: "/foo/bar/thing/James",
method: "get",
status: 404
},
{
url: "/foo/bar/James/123",
method: "get",
status: 404
}
]
},
{
description: "\"options\" is a uri string that has optional express regex constraint",
options: "/foo/bar/{id}(\\d+)/:name?",
action(req, res) {
res.send(`Welcome ${req.params.name ?? "Michael"}. Your ID is ${req.params.id}.`);
},
expected: [
{
url: "/foo/bar/123/James",
method: "get",
status: 200,
response: "Welcome James. Your ID is 123."
},
{
url: "/foo/bar/154",
method: "get",
status: 200,
response: "Welcome Michael. Your ID is 154."
},
{
url: "/foo/bar/thing/James",
method: "get",
status: 404
},
{
url: "/foo/bar/James/123",
method: "get",
status: 404
}
]
},
];

tests.forEach(test => {
Expand Down

0 comments on commit ee80f54

Please sign in to comment.