kruschid / typesafe-routes

Spices up your favorite routing library by adding type safety to plain string-based route definitions.
https://kruschid.github.io/typesafe-routes/
MIT License
102 stars 8 forks source link

Even stronger path typing #52

Open Xample opened 4 months ago

Xample commented 4 months ago

Let's consider this code.

const routes = createRoutes({
    groups: {
        path: ['groups', int('gid')],
        children: {
          users: {
            path: ['users', int('uid').optional],
            query: []
          },
        },
    },
});

routes.render("groups/users", { path: { gid: 123, uid:456 } });

The first argument of the render method is properly typed, i.e. this is a string union dynamically created from the nested properties of the routes. In this way typescript knows about all the possible path and can raise an error if it is wrong.

While string union of paths are much better than plain strings, TS (or webstorm) are unable to refactor those path when for instance we refactor a route property name.

Besides, it is also slightly confusing to write an url path like syntax to target the route to render (according to the feedback of a coworker of mine).

Here are some insets: When I need to type path and when the path is related to an existing structure (interface) I avoid using template string typed path for the following reasons:

For those reasons, I prefer relying on https://www.npmjs.com/package/typed-path?activeTab=readme, I'm using this library to type the translation path in angular. It's not the sexyest syntax but it's much faster and more reliable than the strings.

Here is an example

import {typedPath} from 'typed-path';

type TestType = {
    a: {
        testFunc: () => {result: string};
        b: {
            arrayOfArrays: string[][];
            c: {
                d: number;
            };
        }[];
    };
};

console.log(typedPath<TestType>().a.b[5].c.d.$rawPath);

Behind the hood, the lib uses the interface to constrain the properties path but recursively creates proxies able to keep a ref on the parent to then print the path when we call the reserved property $rawPath (or $path).

The pro with this lib is that refactoring a property either from the path or from the object itself the change will be propagated everywhere. This is really helpful.

So, back to typesafe-routes. I suggest to find a way to achieve the same within the render argument.

Suggestions:

routes.render("groups/users", { path: { gid: 123, uid:456 } });

Could theoretically be rewritten to an array of keys

routes.render(["groups","users"], { path: { gid: 123, uid:456 } });

It should, but I do have a doubt that it will, work with refactoring 2 ways i.e. (from the key to the route and conversely).

Another possible option would be to provide a callback to get the key path from

routes.render((root)=>root.groups.users, { path: { gid: 123, uid:456 } });

While technically it is possible to achieve it in TS, I'm wondering how this would integrate with the current implementation.

Note that using the binding :

routes
    .bind('groups', { path: { gid: 0 } })
    .bind('users', { path: { uid: 0 } })
    .render();

Allows in some case to refactor 2 ways within webstorm (as there is no path but a single key) but we cannot from the route definion find back the usage of the properties within the routes (while it would for instance be the case with typed-path)

kruschid commented 2 weeks ago

Besides, it is also slightly confusing to write an url path like syntax to target the route to render (according to the feedback of a coworker of mine).

Personally, I wouldn't mind a little confusion if the benefits outweigh it. :-) But as you pointed out, this is not the main problem, so let's take a look at your feedback.

* TS will generate all the possible path (string templates) and this slow down the compiler

* TS Has a limit of 2^16 generated string template. 

Right, I left these problems for later. I've read that there are workarounds to deal with these issues. The initial idea behind the string-based notation was to use them as keys for a simple caching concept. However, the mentioned workarounds turned out to be too complicated, and to be honest I didn't find the time to completely understand them. :-)

* The string templates will lose the original context which prevents us from refactoring properly the properties

Yes, accepting your arguments, I'm reintroducing props-based paths (which were featured in the previous versions) in version 12. If we consider your example from the initial post, routes remain the same (except int('uid').optional now becomes int.optional('uid')). Therefore, the rendering of the user's path would work in the following manner:

routes.groups.users.$render({path: {gid: 123, uid: 456}});

TS (or webstorm) are unable to refactor those path when for instance we refactor a route property name.

First tests with Visual Studio Code and TypeScript confirm that segments can be referenced bidirectionally (route tree <-> paths). But I haven't attempted this with WebStorm yet.

However, refactoring doesn't apply to parameter names, meaning they need to be manually renamed. This task is straightforward since the paths can be traced now. There are likely workarounds to address this too, but I prefer to leave it as is. It's a minor compromise for the sake of code simplicity.

For those reasons, I prefer relying on https://www.npmjs.com/package/typed-path?activeTab=readme,

Looks nice. I will test it out in a future project. The previous versions of typesafe-routes as well as the recent v12 also rely on Proxy behind the scenes.

Thank you for your feedback. It took a bit longer than anticipated to address the issues here. I'll also be reviewing your other tickets, though please be aware this might take a while. :-)