-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Changes from 14 commits
962aae0
8029635
5a3fc28
3d5e4b9
6ec83ed
1d6ad57
1ed82db
d98a81e
7691171
c5ef5fb
4e484ce
7098670
c7fb8b8
5d40891
2efe78a
fdbcda7
5bfb400
2b2404c
9c98ce4
fbf166e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,8 +20,8 @@ type PrimitiveJSONValue = string | number | boolean | undefined | null | |
|
||
export interface JSONArray extends Array<JSONValue> {} | ||
|
||
type SerializableJSONValue = | ||
| Symbol | ||
export type SerializableJSONValue = | ||
| symbol | ||
| Set<SuperJSONValue> | ||
| Map<SuperJSONValue, SuperJSONValue> | ||
| undefined | ||
|
@@ -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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I need to There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yep, exactly.
Yep me too. I probably moved some stuff around and this is no longer relevant. I can undo if you want. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oops just remembered, it is only visible in
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should really remove the |
||
[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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding the There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
}) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did test There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think so. But @infomiho can confirm There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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"; | ||
|
@@ -37,7 +37,7 @@ type _WaspEntity = {= crud.entityUpper =} | |
/** | ||
* PUBLIC API | ||
*/ | ||
export namespace {= crud.name =} { | ||
export declare namespace {= crud.name =} { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, can't get this error either. What threw it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nothing, just the recommendations on TypeScript's handbook. If we just do |
||
{=# crud.operations.GetAll =} | ||
export type GetAllQuery<Input extends Payload = never, Output extends Payload = Payload> = {= queryType =}<[_WaspEntityTagged], Input, Output> | ||
{=/ crud.operations.GetAll =} | ||
|
@@ -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. | ||
*/ | ||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
type GetOutput = _WaspEntity | null | ||
export type GetQueryResolved = {= crud.name =}.GetQuery<GetInput, GetOutput> | ||
{=/ overrides.Get.isDefined =} | ||
|
@@ -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 | ||
> | ||
|
@@ -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 | ||
> | ||
|
@@ -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 =} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
export function createOperation (handlerFn) { | ||
return defineHandler(async (req, res) => { | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
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 ofstring
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
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 😂