Skip to content

Types for RequestHandler are Throwing Errors in Typescript #2494

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.

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

Closed
jvanderen1 opened this issue Sep 25, 2021 · 6 comments
Closed

Types for RequestHandler are Throwing Errors in Typescript #2494

jvanderen1 opened this issue Sep 25, 2021 · 6 comments
Labels
bug Something isn't working types / typescript
Milestone

Comments

@jvanderen1
Copy link

jvanderen1 commented Sep 25, 2021

Describe the bug

I am using the the basic example in SvelteKit and when I run the following command:

tsc --noEmit

I get typescript errors.

Reproduction

Take the following file:

import { api } from './_api'
import type { RequestHandler } from '@sveltejs/kit'
import type { Locals } from '$lib/types'

export const get: RequestHandler<Locals> = async (request) => {
  const response = await api(request, `todos/${request.locals.userid}`)

  if (response.status === 404) {
    return { body: [] }
  }

  return response
}

Then run the following command:

tsc --noEmit

I get the following error:

error TS2322: Type '(request: ServerRequest<Locals, unknown>) => Promise<Response | { body: any[]; }>' is not assignable to type 'RequestHandler<Locals, unknown, DefaultBody>'.
  Type 'Promise<Response | { body: any[]; }>' is not assignable to type 'MaybePromise<void | EndpointOutput<DefaultBody>>'.
    Type 'Promise<Response | { body: any[]; }>' is not assignable to type 'Promise<void | EndpointOutput<DefaultBody>>'.
      Type 'Response | { body: any[]; }' is not assignable to type 'void | EndpointOutput<DefaultBody>'.
        Type 'Response' is not assignable to type 'void | EndpointOutput<DefaultBody>'.
          Type 'Response' is not assignable to type 'EndpointOutput<DefaultBody>'.

6 export const get: RequestHandler<Locals> = async (request) => {

Logs

No response

System Info

System:
    OS: macOS 11.5.2
    CPU: (8) x64 Intel(R) Core(TM) i7-4980HQ CPU @ 2.80GHz
    Memory: 1.00 GB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 14.17.6 - ~/.nvm/versions/node/v14.17.6/bin/node
    Yarn: 1.22.11 - /usr/local/bin/yarn
    npm: 6.14.15 - ~/.nvm/versions/node/v14.17.6/bin/npm
  Browsers:
    Chrome: 93.0.4577.82
    Edge: 93.0.961.47
    Firefox: 90.0
    Safari: 14.1.2
  npmPackages:
    @sveltejs/kit: next => 1.0.0-next.168 
    svelte: ^3.34.0 => 3.42.6

Severity

annoyance

Additional Information

No response

@rmunn
Copy link
Contributor

rmunn commented Sep 25, 2021

Probably caused by the changes from #2413.

@benmccann benmccann added this to the 1.0 milestone Sep 25, 2021
@benmccann benmccann added bug Something isn't working help wanted labels Sep 25, 2021
@benmccann
Copy link
Member

@george-lim @ignatiusmb maybe you'd be interested in taking a look at this one?

@baseballyama
Copy link
Member

Hi.
I tried to reproduce this issue.

At first, I tried it with the latest Svelte / kit.

  npmPackages:
    @sveltejs/kit: next => 1.0.0-next.179
    svelte: ^3.34.0 => 3.43.1

After that, I changed these versions according to your System Info.

  npmPackages:
    @sveltejs/kit: 1.0.0-next.168 => 1.0.0-next.168
    svelte: 3.42.6 => 3.42.6

But I couldn't reproduce.
Are the below reproduce steps correct?
(Perhaps I didn't catch the meaning of take.)

npm init svelte@next my-app
# √ Which Svelte app template? » SvelteKit demo app
# √ Use TypeScript? ... Yes
# √ Add ESLint for code linting? ... No
# √ Add Prettier for code formatting? ... No

cd my-app
pnpm i
cat <<"EOL" > src/routes/todos/repl.ts
import { api } from './_api'
import type { RequestHandler } from '@sveltejs/kit'
import type { Locals } from '$lib/types'

export const get: RequestHandler<Locals> = async (request) => {
  const response = await api(request, `todos/${request.locals.userid}`)

  if (response.status === 404) {
    return { body: [] }
  }

  return response
}
EOL
tsc --noEmit

Thank you!

@ignatiusmb
Copy link
Member

I've just had the time to take a deeper look at this issue, and actually, yes, I couldn't reproduce it either. Took exactly the same steps as @baseballyama (thanks!) but installing with npm install, and I see no errors thrown from src/routes/todos/index.json.ts which matches the contents from the description. Also tried to run tsc and svelte-check with no success (no errors) from both outputs.

Closing as it doesn't seem to be an issue, at least an apparent one. Will happily reopen if the author or someone can provide a reproducible example.

@george-lim
Copy link
Contributor

george-lim commented Oct 3, 2021

I also tried @baseballyama's steps, but could not reproduce. No errors being thrown.

@jvanderen1
Copy link
Author

jvanderen1 commented Oct 5, 2021

Root Cause

@ignatiusmb I found out the issue. With ESLint, I was going through my IDE's warnings and issues, I noticed this coming from _api.ts:

ESLint: Missing return type on function.(@typescript-eslint/explicit-module-boundary-types)

I also received this for the data? parameter:

ESLint: 
Don't use `{}` as a type. `{}` actually means "any non-nullish value".
- If you want a type meaning "any object", you probably want `Record<string, unknown>` instead.
- If you want a type meaning "any value", you probably want `unknown` instead.
- If you want a type meaning "empty object", you probably want `Record<string, never>` instead.
(@typescript-eslint/ban-types)

So I changed the the code to the following to add a return type and a better type for data?, which most likely caused the regression we're seeing:

import type { Request } from '@sveltejs/kit'
import type { Locals } from '$lib/types'

/*
	This module is used by the /todos.json and /todos/[uid].json
	endpoints to make calls to api.svelte.dev, which stores todos
	for each user. The leading underscore indicates that this is
	a private module, _not_ an endpoint — visiting /todos/_api
	will net you a 404 response.

	(The data on the todo app will expire periodically; no
	guarantees are made. Don't use it to organise your life.)
*/

const base = 'https://api.svelte.dev'

interface Response {
  status: number
  headers?: { location: string }
  body?: Record<string, unknown>
}

export const api = async (
  request: Request<Locals>,
  resource: string,
  data?: Record<string, unknown>
): Promise<Response> => {
  // user must have a cookie set
  if (!request.locals.userid) {
    return { status: 401 }
  }

  const res = await fetch(`${base}/${resource}`, {
    method: request.method,
    headers: {
      'content-type': 'application/json'
    },
    body: data && JSON.stringify(data)
  })

  // if the request came from a <form> submission, the browser's default
  // behaviour is to show the URL corresponding to the form's "action"
  // attribute. in those cases, we want to redirect them back to the
  // /todos page, rather than showing the response
  if (
    res.ok &&
    request.method !== 'GET' &&
    request.headers.accept !== 'application/json'
  ) {
    return {
      status: 303,
      headers: {
        location: '/todos'
      }
    }
  }

  return {
    status: res.status,
    body: await res.json()
  }
}

Questions

  • Is there better return type interface we can use? I tried Promise<RequestHandler<Locals>> and I get errors.
  • Can the sveltekit example project be updated to add an explicit return type for all exports?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working types / typescript
Projects
None yet
Development

No branches or pull requests

6 participants