ts-rest / ts-rest

RPC-like client, contract, and server implementation for a pure REST API
https://ts-rest.com
MIT License
2.11k stars 91 forks source link

feat (ts-rest/core): allow merging router metadata to route metadata #581

Closed iceStorm closed 2 months ago

iceStorm commented 2 months ago

In our project, we need a mechanism to set metadata for a whole router (and for a sub-router, of course).

Then routes inside a router can inherit this metadata.

Use case:

changeset-bot[bot] commented 2 months ago

πŸ¦‹ Changeset detected

Latest commit: d1f239e858da2c5fd6935d1c89a6d7b6f0a9a0f7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 12 packages | Name | Type | | ----------------------------- | ----- | | @ts-rest/core | Minor | | @ts-rest/serverless | Minor | | @ts-rest/example-contracts | Minor | | @ts-rest/non-strict-mode-test | Minor | | @ts-rest/express | Minor | | @ts-rest/fastify | Minor | | @ts-rest/nest | Minor | | @ts-rest/next | Minor | | @ts-rest/open-api | Minor | | @ts-rest/react-query | Minor | | @ts-rest/solid-query | Minor | | @ts-rest/vue-query | Minor |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

vercel[bot] commented 2 months ago

@iceStorm is attempting to deploy a commit to the ts-rest Team on Vercel.

A member of the Team first needs to authorize it.

iceStorm commented 2 months ago

@Gabrola Hii, could you help to assess this enhancement πŸ˜ƒ

iceStorm commented 2 months ago

Also, we need to access to a route's metadata inside the custom API fetcher callback. Like if a route has metadata with requireAuth: true then we will attach an Authorization header to the request.

Accessing the raw body and content type

Could you help advice some ideas... @Gabrola

Gabrola commented 2 months ago

Also, we need to access to a route's metadata inside the custom API fetcher callback. Like if a route has metadata with requireAuth: true then we will attach an Authorization header to the request.

Accessing the raw body and content type

Could you help advice some ideas... @Gabrola

route is passed to the API fetcher function

iceStorm commented 2 months ago

Thank you for the contribution! Can you please add a test to dsl.spec.ts? Also your branch is quite a bit behind and is conflicting, can you fix that too?

yess, flying on it πŸ˜ƒ

iceStorm commented 2 months ago

You also need to merge the types in ApplyOptions

Updated. I'm not sure if it's aligned or not; it's my first time contributing to OSS. Kindly help to check again πŸ˜ƒ

sonarcloud[bot] commented 2 months ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

nx-cloud[bot] commented 2 months ago

☁️ Nx Cloud Report

CI is running/has finished running commands for commit d1f239e858da2c5fd6935d1c89a6d7b6f0a9a0f7. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

πŸ“‚ See all runs for this CI Pipeline Execution


βœ… Successfully ran 4 targets - [`nx affected --target=build --parallel=3`](https://cloud.nx.app/runs/SuFRsU9vpl?utm_source=pull-request&utm_medium=comment) - [`nx affected --target=test --parallel=3 --ci --code-coverage`](https://cloud.nx.app/runs/5ZhlksKiLV?utm_source=pull-request&utm_medium=comment) - [`nx affected --target=lint --parallel=3`](https://cloud.nx.app/runs/l6P1E6smFp?utm_source=pull-request&utm_medium=comment) - [`nx run-many --target=build --projects=ts-rest*`](https://cloud.nx.app/runs/vcQzWVeZhC?utm_source=pull-request&utm_medium=comment)

Sent with πŸ’Œ from NxCloud.

Gabrola commented 2 months ago

@iceStorm I've just fixed the type merging and add some type tests. Should be good to go. Thank you!

iceStorm commented 2 months ago

@iceStorm I've just fixed the type merging and add some type tests. Should be good to go. Thank you!

Wow, so fast πŸ˜ƒ

Also, sorry to bother you again. May I consult if there is any way to change the type of metadata globally for a project? Like using declare global, declare module...

Let's say we have a common interface for metadata:

interface ApiRouteMetadata {
Β  Β requireAuth?: boolean;
   ...
}

And we want the interface to automatically be inferred when setting metadata for a route or a router. For consistency.

The current idea that pop out of my head is to create a function:

import { AppRoute as TsRestRoute } from '@ts-rest/core';

export type ApiRouteMetadata {
    requireAuth?: boolean;
}

export const SetApiRouteMetadata = (metadata: ApiRouteMetadata) => metadata;

Then call the function when defining a contract:

const contract = c.router(
  {
    getPost: {
      method: "GET",
      path: "/posts/:id",
      responses: {
        200: c.type<{ id: number }>(),
      },
    },
  },
  {
    metadata: SetApiRouteMetadata({ requireAuth: true }),
  }
);

The above solution works. But having a way to force the metadata's shape at a global level to strict what a route's metadata can be would make maintenance easier, IMO.

Gabrola commented 2 months ago

@iceStorm that would be pretty hard to do without extensive refactoring. Module augmentation using declare module wouldn't work because it can only add properties to interfaces, but not change them. Can't think of a better approach than the one you already presented.

export const SetApiRouteMetadata = <const T extends ApiRouteMetadata>(metadata: T) => metadata;

Doing this should be better, because the individual routes will have literal types such as true or false rather than just boolean.

iceStorm commented 2 months ago

@iceStorm that would be pretty hard to do without extensive refactoring. Module augmentation using declare module wouldn't work because it can only add properties to interfaces, but not change them. Can't think of a better approach than the one you already presented.

export const SetApiRouteMetadata = <const T extends ApiRouteMetadata>(metadata: T) => metadata;

Doing this should be better, because the individual routes will have literal types such as true or false rather than just boolean.

Wow, I was curious about how to make the data type from SetApiRouteMetadata a const. Thank you. Learned a new thing πŸ˜ƒ