Skip to content

In CI, check for TS errors in built server code (of example app) by running npx tsc #2672

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

Merged
merged 20 commits into from
May 9, 2025
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ type PrimitiveJSONValue = string | number | boolean | undefined | null

export interface JSONArray extends Array<JSONValue> {}

type SerializableJSONValue =
| Symbol
export type SerializableJSONValue =
| symbol
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just fixing a TS linting warning here, it's because of the same reason why we shouldn't use String instead of string

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, seems right to me...

But I copied this over from superjson (link in the comment above). And they still have Symbol. Can you open an issue there and see whether we're missing something?

Or you'll help them find a bug - win win 😄

Oh, and what threw this error? I'm not getting anything.

Copy link
Member Author

@cprecioso cprecioso Apr 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened a PR, let's see if they have any complaints: flightcontrolhq/superjson#318
The TypeScript Handbook explicitly calls this out so I'm not sure there's a reason for not merging it: https://www.typescriptlang.org/docs/handbook/declaration-files/do-s-and-don-ts.html#number-string-boolean-symbol-and-object

Oh, and what threw this error? I'm not getting anything.

When I wrote this it was because I thought I got the linting warning. Apparently the linter was my own brain 'cause I can't get it again 😂

| Set<SuperJSONValue>
| Map<SuperJSONValue, SuperJSONValue>
| undefined
Expand All @@ -30,14 +30,14 @@ type SerializableJSONValue =
| RegExp

// Here's where we excluded `ClassInstance` (which was `any`) from the union.
type SuperJSONValue =
export type SuperJSONValue =
| JSONValue
| SerializableJSONValue
| SuperJSONArray
| SuperJSONObject

interface SuperJSONArray extends Array<SuperJSONValue> {}
export interface SuperJSONArray extends Array<SuperJSONValue> {}

interface SuperJSONObject {
export interface SuperJSONObject {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to export these types because other types make reference to them.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, which ones?

I can't find any explicit references, so I'm guessing you meant that some types implicitly depend on them when creating declarations (the "type cannot be named without a reference to..." error).

So I've tried removing the exports from SuperJSONArray, SuperJSONValue, and SerializableJsonValue, rebuilding the SDK, and rebuilding the server. Everything seems to work OK.

Not that exporting these types is a problem (perhaps it's a good thing to do even if it doesn't solve anything), but I'd like to understand what's going on.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find any explicit references, so I'm guessing you meant that some types implicitly depend on them when creating declarations (the "type cannot be named without a reference to..." error).

Yep, exactly.

So I've tried removing the exports from SuperJSONArray, SuperJSONValue, and SerializableJsonValue, rebuilding the SDK, and rebuilding the server. Everything seems to work OK.

Yep me too. I probably moved some stuff around and this is no longer relevant. I can undo if you want.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops just remembered, it is only visible in crud-testing:

🐝 --- Building SDK... ------------------------------------------------------------


[  Wasp  ] client/crud/tasks.ts(58,14): error TS4023: Exported variable 'tasks' has or is using name 'SuperJSONArray' from external module "/Users/carlos/Developer/Wasp/wasp/waspc/examples/crud-testing/.wasp/out/sdk/wasp/server/_types/serialization" but cannot be named.
[  Wasp  ] client/crud/tasks.ts(58,14): error TS4023: Exported variable 'tasks' has or is using name 'SuperJSONObject' from external module "/Users/carlos/Developer/Wasp/wasp/waspc/examples/crud-testing/.wasp/out/sdk/wasp/server/_types/serialization" but cannot be named.
[  Wasp  ] client/crud/tasks.ts(58,14): error TS7056: The inferred type of this node exceeds the maximum length the compiler will serialize. An explicit type annotation is needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should really remove the crud-testing app and move the code to todoApp so we don't have to test out with two different apps. We'll do it when we figure out all example apps.

[key: string]: SuperJSONValue
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import { OAuth2Provider, OAuth2ProviderWithPKCE } from "arctic";

export function defineProvider<
OAuthClient extends OAuth2Provider | OAuth2ProviderWithPKCE
OAuthClient extends OAuth2Provider | OAuth2ProviderWithPKCE,
const Id extends string
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the const type parameter makes it output a e.g. {id: "discord"} type instead of a {id: string} one. So then it typechecks correctly for the login provider actions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvement!

>({
id,
displayName,
oAuthClient,
}: {
id: string;
id: Id;
displayName: string;
oAuthClient: OAuthClient;
}) {
Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes make sense but please test them if you haven't (the waspc/examples/todoApp doesn't have CRUD so it might have slipped through the cracks).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did test crud-testing, do you think that covered it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so. crud-testing would be a pretty bad name if it didn't :)

But @infomiho can confirm

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, crud-testing covers the CRUD feature 👍

Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import type {
_{= crud.entityUpper =},
} from "../_types";
import type { Prisma } from "@prisma/client";
import type { Payload } from "../_types/serialization";
import type { Payload, SuperJSONObject } from "../_types/serialization";
import type {
{= crud.entityUpper =},
} from "wasp/entities";
Expand All @@ -37,7 +37,7 @@ type _WaspEntity = {= crud.entityUpper =}
/**
* PUBLIC API
*/
export namespace {= crud.name =} {
export declare namespace {= crud.name =} {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixing a TS lint, namespaces and modules should not be mixed in the runtime time, so we make it only a type thing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, can't get this error either. What threw it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing, just the recommendations on TypeScript's handbook. If we just do export declare there's no output, so no interplay between JS modules and namespaces.

{=# crud.operations.GetAll =}
export type GetAllQuery<Input extends Payload = never, Output extends Payload = Payload> = {= queryType =}<[_WaspEntityTagged], Input, Output>
{=/ crud.operations.GetAll =}
Expand All @@ -61,7 +61,7 @@ export namespace {= crud.name =} {

/**
* PRIVATE API
*
*
* The types with the `Resolved` suffix are the types that are used internally by the Wasp client
* to implement full-stack type safety.
*/
Expand All @@ -79,7 +79,7 @@ export type GetAllQueryResolved = typeof _waspGetAllQuery

{=# crud.operations.Get =}
{=^ overrides.Get.isDefined =}
type GetInput = Prisma.{= crud.entityUpper =}WhereUniqueInput
type GetInput = SuperJSONObject & Prisma.{= crud.entityUpper =}WhereUniqueInput
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Prisma accepts inputs that are not serializable (e.g. functions), so this intersections makes it select only the kinds of inputs that are.
Otherwise it will fail in the following line as Wasp requires only SuperJSON-serializable inputs.

type GetOutput = _WaspEntity | null
export type GetQueryResolved = {= crud.name =}.GetQuery<GetInput, GetOutput>
{=/ overrides.Get.isDefined =}
Expand All @@ -91,7 +91,7 @@ export type GetQueryResolved = typeof _waspGetQuery

{=# crud.operations.Create =}
{=^ overrides.Create.isDefined =}
type CreateInput = Prisma.XOR<
type CreateInput = SuperJSONObject & Prisma.XOR<
Prisma.{= crud.entityUpper =}CreateInput,
Prisma.{= crud.entityUpper =}UncheckedCreateInput
>
Expand All @@ -106,7 +106,7 @@ export type CreateActionResolved = typeof _waspCreateAction

{=# crud.operations.Update =}
{=^ overrides.Update.isDefined =}
type UpdateInput = Prisma.XOR<
type UpdateInput = SuperJSONObject & Prisma.XOR<
Prisma.{= crud.entityUpper =}UpdateInput,
Prisma.{= crud.entityUpper =}UncheckedUpdateInput
>
Expand All @@ -123,7 +123,7 @@ export type UpdateActionResolved = typeof _waspUpdateAction

{=# crud.operations.Delete =}
{=^ overrides.Delete.isDefined =}
type DeleteInput = Prisma.{= crud.entityUpper =}WhereUniqueInput
type DeleteInput = SuperJSONObject & Prisma.{= crud.entityUpper =}WhereUniqueInput
type DeleteOutput = _WaspEntity
export type DeleteActionResolved = {= crud.name =}.DeleteAction<DeleteInput, DeleteOutput>
{=/ overrides.Delete.isDefined =}
Expand Down
7 changes: 3 additions & 4 deletions waspc/data/Generator/templates/sdk/wasp/server/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,14 @@ declare global {
}
}


/**
* Simple helper to give the correct types for Express handlers.
* We define it in the same file as our extension to Request
* so that it is picked up by TypeScript.
*/
export const defineHandler = (
middleware: RequestHandler
) => middleware
export const defineHandler = <T extends RequestHandler>(
middleware: T
): T => middleware

export const sleep = (ms: number): Promise<unknown> => new Promise((r) => setTimeout(r, ms))

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{{={= =}=}}
import { Router, Request, Response, NextFunction } from "express";
import { Router } from "express";

import { ProviderConfig } from "wasp/auth/providers/types";
import type { EmailFromField } from "wasp/server/email/core/types";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export function getLoginRoute() {
return async function login(
req: Request<{ email: string; password: string; }>,
res: Response,
): Promise<Response<{ sessionId: string } | undefined>> {
): Promise<void> {
const fields = req.body ?? {}
ensureValidArgs(fields)

Expand Down Expand Up @@ -54,7 +54,7 @@ export function getLoginRoute() {
user: auth.user,
})

return res.json({
res.json({
sessionId: session.id,
})
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export function getRequestPasswordResetRoute({
return async function requestPasswordReset(
req: Request<{ email: string; }>,
res: Response,
): Promise<Response<{ success: true }>> {
): Promise<void> {
const args = req.body ?? {};
ensureValidEmail(args);

Expand All @@ -43,7 +43,8 @@ export function getRequestPasswordResetRoute({

if (!authIdentity) {
await doFakeWork();
return res.json({ success: true });
res.json({ success: true });
return
}

const providerData = getProviderDataWithPassword<'email'>(authIdentity.providerData);
Expand All @@ -68,6 +69,6 @@ export function getRequestPasswordResetRoute({
throw new HttpError(500, "Failed to send password reset email.");
}

return res.json({ success: true });
res.json({ success: true });
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { HttpError } from 'wasp/server';
export async function resetPassword(
req: Request<{ token: string; password: string; }>,
res: Response,
): Promise<Response<{ success: true }>> {
): Promise<void> {
const args = req.body ?? {};
ensureValidArgs(args);

Expand All @@ -27,7 +27,7 @@ export async function resetPassword(
if (!authIdentity) {
throw new HttpError(400, "Password reset failed, invalid token");
}

const providerData = getProviderDataWithPassword<'email'>(authIdentity.providerData);

await updateAuthIdentityProviderData(providerId, providerData, {
Expand All @@ -38,7 +38,7 @@ export async function resetPassword(
hashedPassword: password,
});

return res.json({ success: true });
res.json({ success: true });
};

function ensureValidArgs(args: object): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export function getSignupRoute({
return async function signup(
req: Request<{ email: string; password: string }>,
res: Response,
): Promise<Response<{ success: true }>> {
): Promise<void> {
const fields = req.body
ensureValidArgs(fields)

Expand Down Expand Up @@ -83,7 +83,8 @@ export function getSignupRoute({
// the email!
if (providerData.isEmailVerified) {
await doFakeWork()
return res.json({ success: true })
res.json({ success: true })
return
}

// TODO: we are still leaking information here since when we are faking work
Expand Down Expand Up @@ -134,7 +135,8 @@ export function getSignupRoute({
// Wasp allows for auto-verification of emails in development mode to
// make writing e2e tests easier.
if (isEmailAutoVerified) {
return res.json({ success: true })
res.json({ success: true })
return
}

const verificationLink = await createEmailVerificationLink(
Expand All @@ -152,7 +154,7 @@ export function getSignupRoute({
throw new HttpError(500, 'Failed to send email verification email.')
}

return res.json({ success: true })
res.json({ success: true })
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { onAfterEmailVerifiedHook } from '../../hooks.js';
export async function verifyEmail(
req: Request<{ token: string }>,
res: Response,
): Promise<Response<{ success: true }>> {
): Promise<void> {
const { token } = req.body;
const { email } = await validateJWT<{ email: string }>(token)
.catch(() => {
Expand All @@ -37,6 +37,6 @@ export async function verifyEmail(

await onAfterEmailVerifiedHook({ req, email, user: auth.user });

return res.json({ success: true });
res.json({ success: true });
};

Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export function createOAuthProviderRouter<OT extends OAuthType, Tokens extends O
url: redirectUrl,
oauth: { uniqueRequestId: oAuthState.state }
})
return redirect(res, redirectUrlAfterHook.toString())
redirect(res, redirectUrlAfterHook.toString())
}),
)

Expand Down Expand Up @@ -113,15 +113,15 @@ export function createOAuthProviderRouter<OT extends OAuthType, Tokens extends O
},
})
// Redirect to the client with the one time code
return redirect(res, redirectUri.toString())
redirect(res, redirectUri.toString())
} catch (e) {
rethrowPossibleAuthError(e)
}
} catch (e) {
console.error(e)
const redirectUri = handleOAuthErrorAndGetRedirectUri(e)
// Redirect to the client with the error
return redirect(res, redirectUri.toString())
redirect(res, redirectUri.toString())
}
}),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export function setupOneTimeCodeRoute(router: Router) {

tokenStore.markUsed(code);

return res.json({
res.json({
sessionId: session.id,
});
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export default defineHandler(async (req, res) => {
user: auth.user,
})

return res.json({
res.json({
sessionId: session.id,
})
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export function getSignupRoute({
rethrowPossibleAuthError(e)
}

return res.json({ success: true })
res.json({ success: true })
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import {
serialize as superjsonSerialize,
} from 'superjson'
import { defineHandler } from 'wasp/server/utils'
{=# isAuthEnabled =}
import { makeAuthUserIfPossible } from 'wasp/auth/user'
{=/ isAuthEnabled =}
Comment on lines +4 to +6
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wasp/auth/user doesn't exist if auth is not enabled


export function createOperation (handlerFn) {
return defineHandler(async (req, res) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { invalidateSession } from 'wasp/auth/session'
export default defineHandler(async (req, res) => {
if (req.sessionId) {
await invalidateSession(req.sessionId)
return res.json({ success: true })
res.json({ success: true })
} else {
throw createInvalidCredentialsError()
}
Expand Down
4 changes: 2 additions & 2 deletions waspc/data/Generator/templates/server/src/routes/auth/me.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import { defineHandler } from 'wasp/server/utils'

export default defineHandler(async (req, res) => {
if (req.user) {
return res.json(superjsonSerialize(req.user))
res.json(superjsonSerialize(req.user))
} else {
return res.json(superjsonSerialize(null))
res.json(superjsonSerialize(null))
}
})
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ import { makeAuthUserIfPossible } from 'wasp/auth/user'
export async function init(server: http.Server): Promise<void> {
// TODO: In the future, we can consider allowing a clustering option.
// Ref: https://github.com/wasp-lang/wasp/issues/1228
const io: ServerType = new Server(server, {

// TODO: Uncomment the type annotation once we make sure that the types between different packages are aligned.
// Ref: https://github.com/wasp-lang/wasp/issues/2726
const io /* : ServerType */ = new Server(server, {
cors: {
origin: config.frontendUrl,
}
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading