Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

RequestEvent and RequestHandlerOutput should be public shapes #4138

Closed
cdcarson opened this issue Feb 26, 2022 · 2 comments · Fixed by #4519
Closed

RequestEvent and RequestHandlerOutput should be public shapes #4138

cdcarson opened this issue Feb 26, 2022 · 2 comments · Fixed by #4519
Labels
feature request New feature or request types / typescript
Milestone
1.0

Comments

@cdcarson
Copy link
Contributor

Describe the problem

This change moved RequestEvent and EndpointOutput into kit/types/private.d.ts from kit/types/index.d.ts, making them "private" rather than reliable.

These two types in particular seem to define the basic contract for endpoints. You get a RequestEvent, you return an EndpointOutput. Unless this contract is going to change radically these types should be made public again.

Describe the proposed solution

Make RequestEvent and EndpointOutput public.

Alternatives considered

  • Keep using the RequestEvent and EndpointOutput types, even though they are ostensibly private.
  • Use the generics given to RequestHandler<Params = Record<string, string>, Output extends Body = Body> . A note about this. The Params generic is of no use to me in userland (though maybe I don't understand it.) For an endpoint at /foo/[fooId] it should be a given that const { fooId } = event.params will always work, be a non-empty string, etc. Is this something internal / adapter-specific that leaked out into the public API?

Importance

would make my life easier

Additional Information

No response
@cdcarson
Copy link
Contributor Author

Relatedly, the shape of EndpointOutput (now RequestHandlerOutput) changed to a MaybePromise. Specifically, the MaybePromise part has been bumped down to RequestHandlerOutput from the RequestHandler function type.

I couldn't find this in the changelog, but here's the commit.

This causes a problem at least in typescript.

import type { RequestHandler, RequestHandlerOutput } from '@sveltejs/kit';

/** 
 * The way one would expect it to work. Error:
 * The return type of an async function or method must be the global Promise<T> type. 
 */
export const get: RequestHandler = async ({request}): RequestHandlerOutput => {
  // do something async here
  return {
    status: 200
  };
}

/**
 * For completeness, I tried returning a promise of a maybe promise.
 * Error: (verbose, basically, that a Promise is not a RequestHandlerOutput)
 * Type '({ request }: RequestEvent<Record<string, string>>) => Promise<RequestHandlerOutput<Body>>' 
 * is not assignable to type 'RequestHandler<Record<string, string>, Body>'. Blah blah blah...
 */
export const post: RequestHandler = async ({request}): Promise<RequestHandlerOutput> => {
  // do something async here
  return {
    status: 200
  }
}

The simple way to fix this is to move the MaybePromise thing back up to RequestHandler type, where it probably belongs. (The function is sync | async, not the output.)

@benmccann benmccann added this to the 1.0 milestone Feb 27, 2022
@Rich-Harris Rich-Harris changed the title RequestEvent and EndpointOutput should be public shapes RequestEvent and RequestHandlerOutput should be public shapes Mar 2, 2022
@benmccann benmccann added the feature request New feature or request label Mar 17, 2022
Closed
@Rich-Harris Rich-Harris mentioned this issue Apr 5, 2022
Merged
5 tasks
@MonoAnji
Copy link

I'm a bit confused why this is closed now even though it's still an issue, I still get warnings when defining hook functions and request handlers because i cannot define the event parameter by it's actual type, which is RequestEvent

consider the following hooks.ts:

import type {GetSession, RequestEvent} from "@sveltejs/kit";

export const getSession: GetSession = async (event: RequestEvent) => {
  //Do anything here where you'd want to use the event with actual type information
}

At least in IntelliJ Idea this yields

TS2459: Module '"@sveltejs/kit"' declares 'RequestEvent' locally, but it is not exported.  index.d.ts(18, 2): 'RequestEvent' is declared here.

Which is correct. So am I not supposed to declare the type of the event parameter? I'd like to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature request New feature or request types / typescript
Projects
None yet
Milestone
1.0
Development

Successfully merging a pull request may close this issue.

move MaybePromise from RequestHandlerOutput to RequestHandler return value sveltejs/kit
3 participants
@cdcarson @benmccann @MonoAnji