-
Notifications
You must be signed in to change notification settings - Fork 18
Fix Prettify #1257
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 Prettify #1257
Conversation
🦋 Changeset detectedLatest commit: 24aae49 The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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.
Pull Request Overview
This PR updates the Prettify utility type so it preserves both plain functions and callable objects (functions with properties), and records that change in the project’s changelog.
- Extend
Prettifyto detect and preserve callable object types - Add a patch entry in the
.changesetfor this fix
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/core/src/types/utils.ts | Add a conditional branch in Prettify to preserve callable objects |
| .changeset/fancy-dancers-dig.md | Add patch changelog entry for fixing Prettify |
Comments suppressed due to low confidence (2)
packages/core/src/types/utils.ts:135
- Update the doc comment to mention that
Prettifynow also preserves callable object types, e.g., change 'preserving function types' to 'preserving function and callable object types'.
* Flattens object types for better IDE display while preserving function types
packages/core/src/types/utils.ts:139
- Add unit tests to verify that
Prettifycorrectly preserves callable object types (functions with properties), ensuring the new branch behaves as intended.
: T extends { (...args: any[]): any } // Check for callable objects
| export type Prettify<T> = T extends { (...args: any[]): any } | ||
| ? T // Preserve callable objects (functions with properties, overloads, etc.) | ||
| : T extends object | ||
| ? { | ||
| [K in keyof T]: T[K] | ||
| } & {} | ||
| : T |
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.
Might we want to have type assertions to verify that this utility type is working as expected?
I did a little research, and here is a more comprehensive utility type Compute from ts-toolbelt lib
https://github.com/millsp/ts-toolbelt/blob/master/sources/Any/Compute.ts, which is well tested but maybe overkill for the use case here.
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.
Vitest can provide type testing. See https://vitest.dev/guide/testing-types.html
|
Just pointing out that the e2e tests are failing |
The only relevant is It's failing cause the conversations API is not working in prod ATM. The other are just flaky errors. |
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.
Left some optional comments to address testing the Prettify<T> type util so it can be ensured it is working as expected.
Description
Makes Prettify type function to preserve the callable objects' types to fix:
Type of change
Code snippets
In case of new feature or breaking changes, please include code snippets.