-
-
Notifications
You must be signed in to change notification settings - Fork 582
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
fix(types): rm ExcludeEmptyObject to fix massively increased type instantiations #3507
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3507 +/- ##
========================================
Coverage 95.58% 95.58%
========================================
Files 155 155
Lines 9357 9357
Branches 2873 2761 -112
========================================
Hits 8944 8944
Misses 413 413 ☔ View full report in Codecov by Sentry. |
c7e2964
to
d8600e3
Compare
@@ -159,7 +160,7 @@ type PathToChain< | |||
} | |||
|
|||
// eslint-disable-next-line @typescript-eslint/no-explicit-any | |||
export type Client<T> = T extends Hono<any, infer S, any> | |||
export type Client<T> = T extends HonoBase<any, infer S, any> |
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.
Weird again.
The reason why I added ExcludeEmptyObject
is that schema type with empty object type broke RPC type.
For example, T
in code below is never
without ExcludeEmptyObject
.
const app = new Hono().route('/', new Hono().get('/foo', c => c.json({ ok: true })))
type T = Client<typeof app>
But, replacing Hono
with HonoBase
in Client
fixed the problem
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.
🤔
const app = new Hono()
.route('/', new Hono().get('/foo', c => c.json({ ok: true })))
type Extract1<T> = T extends Hono<any, infer S, any> ? S : never
type Extract2<T> = T extends HonoBase<any, infer S, any> ? S : never
type R1 = Extract1<typeof app> // {}
type R2 = Simplify<Extract2<typeof app>> // {} | { '/foo': ... }
typeof app
is HonoBase
, but I don't suppose it explains this behaviour
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.
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.
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.
Ah, understand. Thanks!
Hi @m-shaka I can reproduce it. Indeed, it's weird.
How about putting |
We don't need |
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.
LGTM!
Okay! Let's go with it. Thank you. |
…tantiations (honojs#3507) * fix: mv ExcludeEmptyObject to hono-base.ts * refactor: rm ExcludeEmptyObject
The author should do the following, if applicable
bun run format:fix && bun run lint:fix
to format the codeIt's super weird, but if
ExcludeEmptyObject
is outside ofhono-base.ts
, "Type instantiation is excessively deep and possibly infinite." error occurs.Please use this script to check
#3443 (comment)
I don't know the cause at all, it may be a kind of cache issue, though