lukeed / regexparam

A tiny (394B) utility that converts route patterns into RegExp. Limited alternative to `path-to-regexp` 🙇‍♂️
MIT License
567 stars 23 forks source link

Make the library to loosly match at both ends of the URL #10

Closed laszbalo closed 4 years ago

laszbalo commented 4 years ago

I am using this library to convert Express routes to RegExps and then give those RegExps to Workbox. My issue is that Workbox matches the generated RegExps to absolute URLs. Therefore, I added an option to optionally make the matching loose at the start as well:

export default function (str, looseStart, looseEnd) {
    if (str instanceof RegExp) return { keys:false, pattern:str };
    var c, o, tmp, ext, keys=[], pattern='', arr = str.split('/');
    arr[0] || arr.shift();

    while (tmp = arr.shift()) {
        c = tmp[0];
        if (c === '*') {
            keys.push('wild');
            pattern += '/(.*)';
        } else if (c === ':') {
            o = tmp.indexOf('?', 1);
            ext = tmp.indexOf('.', 1);
            keys.push( tmp.substring(1, !!~o ? o : !!~ext ? ext : tmp.length) );
            pattern += !!~o && !~ext ? '(?:/([^/]+?))?' : '/([^/]+?)';
            if (!!~ext) pattern += (!!~o ? '?' : '') + '\\' + tmp.substring(ext);
        } else {
            pattern += '/' + tmp;
        }
    }

    return {
        keys: keys,
        pattern: new RegExp((looseStart ? '' : '^') + pattern + (looseEnd ? '(?=$|\\?|\/)' : '\/?$'), 'i')
    };
}

So,

rgx('/users/:name', true).pattern.test('https://example.com/users/lukeed'); //=> true
rgx('/users/:name', true, true).pattern.test('https://example.com/users/lukeed/repos'); //=> true

Would you consider adding this feature to the library?

lukeed commented 4 years ago

Hey,

No, thank you. This seems like a wildly dangerous idea and could (easily) render your router effectively useless. It also means that the base parameter is meaningless, since route-patterns could now match anywhere within the entire URL. No point of a base path with that behavior.

Appreciate it though :)

laszbalo commented 4 years ago

Thanks for taking the time to look into this. I understand that you don't want this into your library.

Personally had similar feelings about this proposal, but for my defense, this is how Workbox actually works for same origin requests.

From their docs:

new RegExp('/styles/.*\\.css') // NOTE: The origin where Workbox is running is example.com

The above RegExp will match all of the following URLs:

However, I did not think through the possible implications of this change could have on people who use your library differently. (in Node.js, etc)

lukeed commented 4 years ago

No worries!

Yes, but Workbox is primarily concerned with assets and directory relationships/structures, typically with extension information. As with your example, it says "any CSS files inside styles", which allows it to be anywhere so long as it's a CSS file directly within styles.

That said, this will also match:

example.com/foo/styles/main.css/oops

But that's fine for Workbox because that doesn't/won't happen 99.999999% of the time