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

Add ids to Participant example in README #162

Closed
wants to merge 1 commit into from

Conversation

mikhail
Copy link

@mikhail mikhail commented Apr 16, 2023

TypeScript checking failed when copy/pasting the README example after installing the latest version

@Drarig29
Copy link
Owner

Hey @mikhail! Thanks for the PR!

However, I'm sorry I can't accept this pull request as is because it's actually the type definition for manager.update.match() that is incorrect. This function works without giving participant IDs, and I want it that way.

So I don't want to show an example with something that is not required. This example should stay minimal.

Do you still want to work on fixing the type definition in this PR?
Basically, you should do a change like this (and update the rest of the code accordingly):

diff --git a/src/update.ts b/src/update.ts
index 3934e35..023e4fb 100644
--- a/src/update.ts
+++ b/src/update.ts
@@ -4,6 +4,10 @@ import { BaseUpdater } from './base/updater';
 import { ChildCountLevel } from './types';
 import * as helpers from './helpers';
 
+type DeepPartial<T> = T extends object ? {
+    [P in keyof T]?: DeepPartial<T[P]>;
+} : T;
+
 export class Update extends BaseUpdater {
 
     /**
@@ -13,7 +17,7 @@ export class Update extends BaseUpdater {
      *
      * @param match Values to change in a match.
      */
-    public async match<M extends Match = Match>(match: Partial<M>): Promise<void> {
+    public async match<M extends Match = Match>(match: DeepPartial<M>): Promise<void> {
         if (match.id === undefined)
             throw Error('No match id given.');

@Drarig29
Copy link
Owner

If you don't want to work on this, it's perfectly fine! I can also close the PR and work on it. 👌

@Drarig29
Copy link
Owner

Drarig29 commented Apr 16, 2023

It's more or less related to #105.
I could take this opportunity to use zod for runtime type checking.

Copy link
Owner

@Drarig29 Drarig29 left a comment

Choose a reason for hiding this comment

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

I don't want to let people think they will need to know the opponents IDs, since it's implicitly known already.

@Drarig29 Drarig29 closed this Apr 18, 2023
@Drarig29
Copy link
Owner

Drarig29 commented Apr 18, 2023

I fixed it in this commit: 5b60a8d

@Drarig29
Copy link
Owner

Released in v1.5.8

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