Pass more matchCallback return values through to handlerCallback

[jeffposnick]

R: @philipwalton

Fixes #2079

This change results in more return values from the matchCallback being passed through to the handlerCallback as the params option.

(There is still some special-case logic for {} and [] being treated as {params: undefined}, matching the existing v4 behavior. It's implemented this way to make it easier to deal with a matchCallback that uses a RegExp with capture groups.)

[philipwalton]

I'm mostly OK with this change, but (and I remember thinking about this when converting the code to TypeScript) I think the intention of the match callback is to be boolean-ish. That is, I think in most cases the function answers the question: Does this route match? Yes or no.

With this change a return value of true will get passed as the params object, which feels like it goes against the intention of the params property (though it probably won't have any actual consequences).

I do see a use case for returning a string and having that passed to params though.

[jeffposnick]

With this change a return value of true will get passed as the params object, which feels like it goes against the intention of the params property (though it probably won't have any actual consequences).

Would you rather I change it so that true doesn't get passed through, or are you just cool with this PR as-is?

[philipwalton]

I think so. Let's make is so that:

• Any falsy value means no match.
• A strict true value means a match but no params.
• Any non-true, truthy value means params.

I'm also fine with dropping the support for removing empty array/objects, but maybe there's good reason to keep that?

It's implemented this way to make it easier to deal with a matchCallback that uses a RegExp with capture groups.

I don't really understand why this is the case. Wouldn't RegExp.exec() and friends return falsy when there's no match? In what case is there a match but empty capture groups that we want to ignore?

[jeffposnick]

Okay, please take another look with the latest changes.

[philipwalton]

LGTM with one minor typo I saw.

[workbox-pr-bot]

PR-Bot Size Plugin

Changed File Sizes

File	Before	After	Change	GZipped
packages/workbox-cli/build/app.js	4.16 KB	4.15 KB	-0%	1.64 KB
packages/workbox-routing/build/workbox-routing.prod.js	3.41 KB	3.46 KB	+1%	1.49 KB
packages/workbox-webpack-plugin/build/inject-manifest.js	4.68 KB	4.67 KB	-0%	1.59 KB

New Files

No new files have been added.

All File Sizes

View Table

File	Before	After	Change	GZipped
packages/workbox-background-sync/build/workbox-background-sync.prod.js	3.86 KB	3.86 KB	0%	1.59 KB
packages/workbox-broadcast-update/build/workbox-broadcast-update.prod.js	1.92 KB	1.92 KB	0%	959 B
packages/workbox-build/build/_types.js	41 B	41 B	0%	61 B
packages/workbox-build/build/index.js	3.30 KB	3.30 KB	0%	1.31 KB
packages/workbox-cacheable-response/build/workbox-cacheable-response.prod.js	594 B	594 B	0%	354 B
packages/workbox-cli/build/app.js	4.16 KB	4.15 KB	-0%	1.64 KB
packages/workbox-cli/build/bin.js	940 B	940 B	0%	502 B
packages/workbox-core/build/workbox-core.prod.js	5.94 KB	5.94 KB	0%	2.46 KB
packages/workbox-expiration/build/workbox-expiration.prod.js	2.94 KB	2.94 KB	0%	1.27 KB
packages/workbox-google-analytics/build/workbox-offline-ga.prod.js	1.95 KB	1.95 KB	0%	913 B
packages/workbox-navigation-preload/build/workbox-navigation-preload.prod.js	660 B	660 B	0%	324 B
packages/workbox-precaching/build/workbox-precaching.prod.js	4.25 KB	4.25 KB	0%	1.70 KB
packages/workbox-range-requests/build/workbox-range-requests.prod.js	1.57 KB	1.57 KB	0%	797 B
packages/workbox-routing/build/workbox-routing.prod.js	3.41 KB	3.46 KB	+1%	1.49 KB
packages/workbox-strategies/build/workbox-strategies.prod.js	4.10 KB	4.10 KB	0%	1.13 KB
packages/workbox-streams/build/workbox-streams.prod.js	1.46 KB	1.46 KB	0%	720 B
packages/workbox-sw/build/workbox-sw.js	1.34 KB	1.34 KB	0%	747 B
packages/workbox-webpack-plugin/build/generate-sw.js	3.71 KB	3.71 KB	0%	1.40 KB
packages/workbox-webpack-plugin/build/index.js	349 B	349 B	0%	255 B
packages/workbox-webpack-plugin/build/inject-manifest.js	4.68 KB	4.67 KB	-0%	1.59 KB
packages/workbox-window/build/workbox-window.dev.umd.js	31.88 KB	31.88 KB	0%	8.10 KB
packages/workbox-window/build/workbox-window.prod.umd.js	4.46 KB	4.46 KB	0%	1.84 KB

Workbox Aggregate Size Plugin

3.49KB gzip'ed (23% of limit)
7.8KB uncompressed

[coveralls]

Coverage increased (+0.08%) to 78.731% when pulling 12302a1 on matchCb-return-values into b0825d7 on master.