kit icon indicating copy to clipboard operation
kit copied to clipboard

Colocate matchers (+matchers.js)

Open stephane-vanraes opened this issue 2 years ago • 9 comments

Describe the problem

Currently matchers are placed in a seperate folder, away from the routes.

Describe the proposed solution

Place matchers in +page.js (or +page.server js) or even +matcher.js.

Recurring matchers can still be placed in $lib and imported/exported in the page file.

This change would also remove the need to name the matcher and simplify the file name.

Alternatives considered

Keep the status quo

Importance

nice to have

Additional Information

No response

stephane-vanraes avatar Sep 10 '22 20:09 stephane-vanraes

As in #4539, matchers in +page.js or +page.server.js is a no-go. I have slightly less strong feelings about +matcher.js, but it's still not immediately clear how to deal with path segments that contain multiple params.

Conduitry avatar Sep 10 '22 21:09 Conduitry

I feel like some extra background can be beneficial. I am trying to solve the 'translated routes' problem and came to this solution:

route: /routes/[about=about]/+page.svelte and in /matchers/about.js

import canBeTranslatedAs from '...';
export function match(param) {
  return canBeTranslatedAs(param, "about");
}

With this change the files would become /routes/[about]/+page.svelte /routes/[about]/+matcher.js

Which I think would be an improvement.

stephane-vanraes avatar Sep 10 '22 22:09 stephane-vanraes

As in #4539, matchers in +page.js or +page.server.js is a no-go. I have slightly less strong feelings about +matcher.js, but it's still not immediately clear how to deal with path segments that contain multiple params.

What about +match.name.ts (or ,+matcher.name.ts) where name is the name of the param? It would allow multiple matchers and drop the awkward [name=matcher_type] syntax. Alternatively: allow one +matchers.ts with multiple exports named after the param names.

We also use matchers for translations and having the [name=matcher_type] syntax in every folder is quite distracting.

aloker avatar Sep 11 '22 05:09 aloker

As in #4539, matchers in +page.js or +page.server.js is a no-go.

Yes, I forgot about that one again, thanks for not reflexively closing this issue.

but it's still not immediately clear how to deal with path segments that contain multiple params.

Not sure what you mean here with "multiple params", wouldn't this work exactly as it does now?

/matchers
    categories.js
    uuid.js
/routes
  /[category=categories]
     /[id=uuid]
         +page.svelte

would become:

/routes
   /[category]
      /[id]
         +matcher.js    //uuid 
         +page.svelte
      +matcher.js     // categories

Unless I am missing a usage of matchers that I am not fully aware off.

I reckon this change might lead to more files in case of recurring matchers, with partially duplicate code though, but those could be simple one liners (but with correct syntax):

export { uuid as match } from '$lib/matchers;

(as an aside: could the matcher be export default?)

stephane-vanraes avatar Sep 11 '22 16:09 stephane-vanraes

What about +match.name.ts (or ,+matcher.name.ts) where name is the name of the param? It would allow multiple matchers and drop the awkward [name=matcher_type] syntax. Alternatively: allow one +matchers.ts with multiple exports named after the param names.

Not sure where this goes, the +matcher.js would be specific to one route and hence one param, so the dot name seems unnecessary. Unless, as in my previous post, I missed something obvious.

stephane-vanraes avatar Sep 11 '22 16:09 stephane-vanraes

but it's still not immediately clear how to deal with path segments that contain multiple params.

Not sure what you mean here with "multiple params", wouldn't this work exactly as it does now?

He is talking about routes/[param1=matcher1]-[param2=matcher2].[param3=matcher3]/+page.svelte.

Tho that could be resolved with:

// routes/[param1=matcher1]-[param2=matcher2].[param3=matcher3]/+matcher.js

export const param1 = ...;
export const param2 = ...;
export const param3 = ...;

One question that comes to my mind, how would the matchers be deduped? If you have 10 routes with a numeric param, you have to create 10 matchers with the same logic.

Even if you do something like:

// lib/my-matcher.js
export const numeric = ...;

// routes/thing1/[id]/+matcher.js
export { numeric as match } from '$lib/my-matchers';

// routes/thing2/[id]/+matcher.js
export { numeric as match } from '$lib/my-matchers';

// routes/thingN/[id]/+matcher.js
export { numeric as match } from '$lib/my-matchers';

The client would need to load the same matcher 10 times, I guess (not sure about that anymore).

PatrickG avatar Sep 11 '22 17:09 PatrickG

He is talking about routes/[param1=matcher1]-[param2=matcher2].[param3=matcher3]/+page.svelte.

Tho that could be resolved with:

// routes/[param1=matcher1]-[param2=matcher2].[param3=matcher3]/+matcher.js

export const param1 = ...;
export const param2 = ...;
export const param3 = ...;

I would rather change the signature of the matcher function to take an object (which I believe to be better in general anyway because of the extensibility):

export function match({ param1, param2, param3 }) {

}

One question that comes to my mind, how would the matchers be deduped? If you have 10 routes with a numeric param, you have to create 10 matchers with the same logic.

Even if you do something like:

// lib/my-matcher.js
export const numeric = ...;

// routes/thing1/[id]/+matcher.js
export { numeric as match } from '$lib/my-matchers';

// routes/thing2/[id]/+matcher.js
export { numeric as match } from '$lib/my-matchers';

// routes/thingN/[id]/+matcher.js
export { numeric as match } from '$lib/my-matchers';

The client would need to load the same matcher 10 times.

Yes this could be problematic, but as you show this feels to me to as not that bad, the actual code of numeric would not be duped, just the part where it is being invoked. I do not know how this is resolved in the current implementation but would expect to find duplication there as well regardless. I have full confidence in the core team that they would be able to solve this in a good way :)

Another point I thought of was how to do multiple paths with the same param but a different matcher:

/routes
  /product
    /[slug=integer]
    /[slug=string]

But that is a mere naming issue, just call the params something differently:

/routes
  /product
    /[id]
    /[name]

stephane-vanraes avatar Sep 11 '22 17:09 stephane-vanraes

The route-local matchers should be available in addition to the current system. So common matchers like int, uuid etc could still go to the params directory. For us at least, it's the route specific matchers (namely those for localization) that add unwanted noise.

aloker avatar Sep 11 '22 20:09 aloker

This is a dupe of https://github.com/sveltejs/kit/issues/5298#issuecomment-1238288084. Ultimately I don't think matchers are the right solution for internationalising routes, even if they're the best tool we have right now

Rich-Harris avatar Sep 12 '22 22:09 Rich-Harris