Skip to content
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 batchRenderUserMessages() type #4198

Merged
merged 1 commit into from
Mar 7, 2024
Merged

Conversation

lasryaric
Copy link
Contributor

Description

batchRenderUserMessages() is not returning a UserMessageType compliant data structure, so this PR fixes it.
The product work and it fully compiles, so I assume it's ok.
The only real risk is the .workspaces that I have removed, but typescript should have raised an error if it was used.

Risk

Deploy Plan

@lasryaric lasryaric requested a review from spolu March 7, 2024 14:05
@@ -267,7 +270,7 @@ export async function batchRenderUserMessages(messages: Message[]) {
email: userMessage.userContextEmail,
profilePictureUrl: userMessage.userContextProfilePictureUrl,
},
};
} satisfies UserMessageType;
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From GPT4:

The satisfies Keyword: The satisfies keyword is a newer addition to TypeScript, introduced in TypeScript 4.9, aimed at type checking rather than type assertion. It allows you to check if a value satisfies a particular type without changing the value's type. This is particularly useful for object literals and ensuring they adhere to a specific shape or interface without changing their inferred type.

Without that, the type system was not able to infer the right type in this code. So I initially added it to highlight what was wrong, and realized that it helped the type checker inferring it correctly.

@spolu
Copy link
Contributor

spolu commented Mar 7, 2024

Can we also do the same for agentMessage right next to it?

@lasryaric
Copy link
Contributor Author

Can we also do the same for agentMessage right next to it?

I will send another PR for that.

@lasryaric lasryaric merged commit b9ec45a into main Mar 7, 2024
5 checks passed
@lasryaric lasryaric deleted the aric-renderUserMessage-fix branch March 7, 2024 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants