Skip to content

JSONObject type used by EndpointOutput seems wrong #3765

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
biltongza opened this issue Feb 7, 2022 · 3 comments
Closed

JSONObject type used by EndpointOutput seems wrong #3765

biltongza opened this issue Feb 7, 2022 · 3 comments

Comments

@biltongza
Copy link

biltongza commented Feb 7, 2022

Describe the bug

I got excited about shadow endpoints and just tried to migrate one of my endpoints.

It seemed simple enough, this is what I started with:

export const get: RequestHandler = function () {
	const posts = fs
		.readdirSync(`src/posts`)
		.filter((fileName) => /.+\.md$/.test(fileName))
		.map((fileName) => {
			const { metadata } = process(`src/posts/${fileName}`);
			return metadata;
		});
	// sort the posts by create date.
	posts.sort((a, b) => dayjs(a.date, DateFormat).valueOf() - dayjs(b.date, DateFormat).valueOf());
	const body = JSON.stringify(posts);

	return {
		body
	};
};

(basically verbatim from the sveltekit blog demo)

From the docs, it looks like I can just return posts directly in body, and then export let posts in my .svelte file. So that's what I tried.

export const get: RequestHandler = function () {
	const posts = fs
		.readdirSync(`src/posts`)
		.filter((fileName) => /.+\.md$/.test(fileName))
		.map((fileName) => {
			const { metadata } = process(`src/posts/${fileName}`);
			return metadata;
		});
	// sort the posts by create date.
	posts.sort((a, b) => dayjs(a.date, DateFormat).valueOf() - dayjs(b.date, DateFormat).valueOf());

	return {
		body: {
			posts,
		}
	};
};

Typescript is unhappy with this:

/Users/logan/projects/ldam.co.za/frontend/src/routes/blog/index.ts:7:14
Error: Type '() => { body: { posts: BlogMetadata[]; }; }' is not assignable to type 'RequestHandler<Body>'.
  Type '{ body: { posts: BlogMetadata[]; }; }' is not assignable to type 'MaybePromise<Either<EndpointOutput<Uint8Array> | EndpointOutput<string> | EndpointOutput<number> | EndpointOutput<false> | ... 5 more ... | EndpointOutput<...>, Fallthrough>>'.
    Type '{ body: { posts: BlogMetadata[]; }; }' is not assignable to type 'Only<Fallthrough, EndpointOutput<Uint8Array> | EndpointOutput<string> | EndpointOutput<number> | ... 6 more ... | EndpointOutput<...>>'.
      Property 'fallthrough' is missing in type '{ body: { posts: BlogMetadata[]; }; }' but required in type '{ fallthrough: true; }'. 

export const get: RequestHandler = function () {
        const posts = fs

That led me on a short goose chase through the typings to try figure out why I needed to specify a fallthrough.

EndpointOutput looks like this:

export interface EndpointOutput<Output extends Body = Body> {
	status?: number;
	headers?: Headers | Partial<ResponseHeaders>;
	body?: Output;
}

And body is defined like so:

type Body = JSONValue | Uint8Array | ReadableStream | import('stream').Readable;

The problem comes to how JSONValue is defined:

export type JSONObject = { [key: string]: JSONValue };
export type JSONValue = string | number | boolean | null | ToJSON | JSONValue[] | JSONObject;

The problem in my case stems from my return type not implementing an index signature, which I only discovered after explicitly trying to make a EndpointOutput object myself:

const result: EndpointOutput = {
	body: {
		posts
	},
};
/Users/logan/projects/ldam.co.za/frontend/src/routes/blog/index.ts:18:3
Error: Type '{ posts: BlogMetadata[]; }' is not assignable to type 'Body'.
  Types of property 'posts' are incompatible.
    Type 'BlogMetadata[]' is not assignable to type 'JSONValue'.
      Type 'BlogMetadata[]' is not assignable to type 'JSONValue[]'.
        Type 'BlogMetadata' is not assignable to type 'JSONValue'.
          Type 'BlogMetadata' is not assignable to type 'JSONObject'.
            Index signature for type 'string' is missing in type 'BlogMetadata'. 
        const result: EndpointOutput = {
                body: {
                        posts

The example in the docs kind of makes it look like you can return any object you like:

/** @type {import('@sveltejs/kit').RequestHandler} */
export async function get({ params }) {
	// `params.id` comes from [id].js
	const item = await db.get(params.id);

	if (item) {
		return {
			body: { item }
		};
	}

	return {
		status: 404
	};
}

item in this case looks like any plain old javascript object, much like my object(array) is.

Either the docs need to specify what constraints the returned object must satisfy, or the type of JSONObject is wrong (especially since index signatures are just a Typescript thing). Indeed, ignoring typescript errors here and running the project, it works fine, so I'm pretty sure the JSONObject constraint is too strict.

If JSONObject is instead defined like so:

export type JSONObject = Record<string, unknown>;

This works fine, but perhaps the index signature approach was taken for a reason.

Reproduction

See above

Logs

No response

System Info

System:
    OS: macOS 12.1
    CPU: (10) arm64 Apple M1 Pro
    Memory: 280.09 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.13.0 - ~/.nvm/versions/node/v16.13.0/bin/node
    npm: 8.1.0 - ~/.nvm/versions/node/v16.13.0/bin/npm
  Browsers:
    Chrome: 97.0.4692.99
    Firefox: 96.0.3
    Firefox Developer Edition: 96.0
    Safari: 15.2
  npmPackages:
    @sveltejs/kit: ^1.0.0-next.231 => 1.0.0-next.260 
    svelte: ^3.46.4 => 3.46.4       

Severity

annoyance

Additional Information

No response

@ignatiusmb
Copy link
Member

May be a duplicate, see #1997 (comment)

What does BlogMetadata looks like? "See above" is not a reproduction

@biltongza
Copy link
Author

Yep, that's exactly my issue. It's an interface, sorry I missed that detail.

I wonder if it's worth noting this in the docs.

@Rich-Harris
Copy link
Member

Looks like this is indeed a duplicate so I'll close in favour of #1997

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants