Financial-Times / n-service-worker

❌ DECOMMISSIONED Global service worker component for next.ft.com
5 stars 2 forks source link

Massive simplification of module? #84

Closed leggsimon closed 7 years ago

leggsimon commented 7 years ago

@tavvy I'm just double checking incase I've overlooked something but this function keyMatch in this module

function keyMatch (map, string) {
    // This would be better written as a for..of loop, but that would break the
    // minifyify process in the build.
    const entriesIterator = map.entries();
    let item = entriesIterator.next();
    const matches = [];
    while (!item.done) {
        const pattern = new RegExp(item.value[0]);
        if (pattern.test(string)) {
            matches.push(item.value[1]);
        }
        item = entriesIterator.next();
    }
    return matches;
};

Doesn't this just do the same as

map.get(string)

or if it must return an array (which seems odd because it would only have one result), do this?

return map.get(string) ? [map.get(string)] : [];
leggsimon commented 7 years ago

Ohhh hang on, I think I know why I'm wrong here. This is so that a Map containing

['key1', 'value1'],
['key12', 'value12'],
['key123', 'value123'],
['key1234', 'value1234'],
['key12345', 'value12345']

if we used map.get('key12345') we would get one result, whereas we currently would get all of those back since all the keys are included in 'key12345'

tavvy commented 7 years ago

I can't remember writing this an looked through the commit history.

I lifted it from the sw Router and made it a shared util so it could be used for the msg router I wrote (which is basically a very stripped down version of the Router).

So I never looked a the code in too much detail. Another thing to note is that it is doing a regexp.test to find a match aswell so that routes like /* work or /^blah.