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 TypeScript Type Definitions: Remove Incorrect Generic Usage of Uint8Array #858

Closed
wants to merge 2 commits into from

Conversation

Sn0w3y
Copy link

@Sn0w3y Sn0w3y commented Jan 8, 2025

Description

Issue Overview

The uuid package contains TypeScript type definitions that incorrectly utilize Uint8Array as a generic type. Specifically, instances such as Uint8Array<ArrayBuffer> are invalid because Uint8Array is not a generic type in TypeScript. This misuse leads to TypeScript compilation errors, hindering developers from successfully building projects that depend on uuid.

Error Messages Encountered

error TS2315: Type 'Uint8Array' is not generic.

These errors are triggered in the following type definition files:

  • dist/cjs/parse.d.ts
  • dist/cjs/v35.d.ts

Changes Implemented

  1. src/parse.ts

    • Added Explicit Return Type:
      function parse(uuid: string): Uint8Array {
    • Typed Variable v:
      let v: number;
    • Added JSDoc Comments:
      /**
       * Parses a UUID string into a Uint8Array.
       *
       * @param uuid - The UUID string to parse.
       * @returns A Uint8Array representing the binary form of the UUID.
       * @throws TypeError if the UUID is invalid.
       */
  2. src/v35.ts

    • Added Explicit Return Types:
      export function stringToBytes(str: string): Uint8Array {
      export default function v35(
        version: 0x30 | 0x50,
        hash: HashFunction,
        value: string | Uint8Array,
        namespace: UUIDTypes,
        buf?: Uint8Array,
        offset?: number
      ): string | Uint8Array {
    • Added JSDoc Comments:
      /**
       * Converts a string to a Uint8Array.
       *
       * @param str - The string to convert.
       * @returns A Uint8Array representing the binary form of the string.
       */
      /**
       * Generates a version 3 or 5 UUID based on the provided parameters.
       *
       * @param version - The UUID version (0x30 for v3, 0x50 for v5).
       * @param hash - The hash function to use.
       * @param value - The value to hash.
       * @param namespace - The namespace UUID.
       * @param buf - Optional buffer to write the UUID into.
       * @param offset - Optional offset in the buffer.
       * @returns A string representation of the UUID or a Uint8Array if a buffer is provided.
       */

Rationale

  • Type Safety: Explicitly defining return types ensures that the functions adhere strictly to their intended types, preventing TypeScript from misinterpreting or inferring incorrect types.

  • Compiler Compliance: Removing the incorrect generic usage of Uint8Array eliminates TypeScript compilation errors, facilitating smoother integration of the uuid package in TypeScript projects.

  • Enhanced Documentation: Adding JSDoc comments improves code readability and maintainability, providing clear guidance on the purpose and usage of each function.

Impact

These changes correct the TypeScript type definitions, ensuring that Uint8Array is used properly without generics. This resolves the TypeScript compilation errors and enhances the overall type safety and developer experience when using the uuid package.


Conclusion

This PR addresses and resolves the incorrect generic usage of Uint8Array in the uuid package's TypeScript definitions by adding explicit return types and removing invalid generics. These changes enhance type safety, eliminate compilation errors, and improve the overall reliability of the uuid package for TypeScript users.


Probably fixes: #856

Copy link

@FernandoBonfimArctica FernandoBonfimArctica left a comment

Choose a reason for hiding this comment

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

Nice

@Sn0w3y
Copy link
Author

Sn0w3y commented Jan 8, 2025

@FernandoBonfimArctica did you Check and Test?

@broofa
Copy link
Member

broofa commented Jan 9, 2025

I'm considering reverting the typescript dependency in this project back to 5.0.4, in which case I expect this PR won't be needed.

That said, if I were to take this PR ...

  • Remove JSDoc (it's redundant with the README which, for better or worse, is the canonical location for docs at this time.)
  • Uint8Array type fixes would need to be made for all APIs, (v1, v4, v6, v7)
  • Needs a unit test of some sort to demonstrate the issue / verify the code here fixes the problem

@broofa
Copy link
Member

broofa commented Jan 9, 2025

Closing in preference to #860. Thanks for putting this up, though!

@broofa broofa closed this Jan 9, 2025
@Sn0w3y Sn0w3y deleted the patch-1 branch January 9, 2025 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants