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

Type alias guards reused in places that match the wrong type. #189

Open
rhys-vdw opened this issue Apr 8, 2022 · 0 comments
Open

Type alias guards reused in places that match the wrong type. #189

rhys-vdw opened this issue Apr 8, 2022 · 0 comments

Comments

@rhys-vdw
Copy link
Owner

rhys-vdw commented Apr 8, 2022

Reported by @kernwig: #183 (comment)

This did indeed "break" my fix.

The change here adds these... useless to me but no objection:

+export function isCognitoId(obj: any, _argumentName?: string): obj is CognitoId {
+  return typeof obj === "string";
+}
+
+export function isDeviceId(obj: any, _argumentName?: string): obj is DeviceId {
+  return typeof obj === "string";
+}

But then every single string comparison is again calling isCognitoId. Here's a example diff:

 export function isRelationship(obj: any, _argumentName?: string): obj is Relationship {
   return (
     ((obj !== null && typeof obj === "object") || typeof obj === "function") &&
-    typeof obj.userId === "string" &&
-    typeof obj.deviceId === "string"
+    (isCognitoId(obj.userId) as boolean) &&
+    (isCognitoId(obj.deviceId) as boolean)
   );
 }

isCognitoId(obj.deviceId) is functional, but absolutely misleading. I don't think this in an opinionated issue. It isn't just string, every primitive type now calls the function for the first alias to it.

Please either call the correct guard (isDeviceId), or restore my fix to use typeof for primitives within objects.

Possible solutions:

  1. Just never reuse generated type guards at all, instead generate a giant function for the full depth of objects.
  2. Never reuse primitive type guards (similar to @kernwig's fix, but only affecting reuse of guards, not actual generation of guards).
  3. Be aware of the specific symbol that is being queried and select the right type guard

I'm pretty flat out at present so I won't be doing this one unless the mood takes me, but feel free to chime in if you have thoughts.

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

No branches or pull requests

1 participant