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

Suggestion : Enhance Code Readability by Renaming Generic Type Parameters to Descriptive Names #538

Open
damien-schneider opened this issue Nov 21, 2024 · 0 comments

Comments

@damien-schneider
Copy link
Contributor

Description:

Currently, the codebase contains complex type handling inherited from supabase-js, using single-letter generic type parameters like S, T, Q, and R. While this may be acceptable for simple generics, it can make the code difficult to understand and navigate, particularly for new contributors.

Example of the Current Implementation:

/**
 * Hook to execute a INSERT mutation
 *
 * @param {PostgrestQueryBuilder<S, T>} qb PostgrestQueryBuilder instance for the table
 * @param {Array<keyof T['Row']>} primaryKeys Array of primary keys of the table
 * @param {string | null} query Optional PostgREST query string for the INSERT mutation
 * @param {Omit<UsePostgrestMutationOpts<S, T, 'Insert', Q, R>, 'mutationFn'>} [opts] Options to configure the hook
 */
function useInsertMutation<
  S extends GenericSchema,
  T extends GenericTable,
  RelationName,
  Re = T extends { Relationships: infer R } ? R : unknown,
  Q extends string = '*',
  R = GetResult<S, T['Row'], RelationName, Re, Q extends '*' ? '*' : Q>,
>(
  qb: PostgrestQueryBuilder<S, T, Re>,
  primaryKeys: (keyof T['Row'])[],
  query?: Q | null,
  opts?: Omit<
    UsePostgrestMutationOpts<S, T, RelationName, Re, 'Insert', Q, R>,
    'mutationFn'
  >,
) {

Proposed Improvement:

/**
 * Hook to execute a INSERT mutation
 *
 * @param {PostgrestQueryBuilder<Schema, Table>} qb PostgrestQueryBuilder instance for the table
 * @param {Array<keyof Table['Row']>} primaryKeys Array of primary keys of the table
 * @param {string | null} query Optional PostgREST query string for the INSERT mutation
 * @param {Omit<UsePostgrestMutationOpts<Schema, Table, 'Insert', Query, Result>, 'mutationFn'>} [opts] Options to configure the hook
 */
function useInsertMutation<
  Schema extends GenericSchema,
  Table extends GenericTable,
  RelationName,
  Relation = Table extends { Relationships: infer R } ? R : unknown,
  Query extends string = '*',
  Result = GetResult<
    Schema,
    Table['Row'],
    RelationName,
    Relation,
    Query extends '*' ? '*' : Query
  >,
>(
  qb: PostgrestQueryBuilder<Schema, Table, Relation>,
  primaryKeys: (keyof Table['Row'])[],
  query?: Query | null,
  opts?: Omit<
    UsePostgrestMutationOpts<
      Schema,
      Table,
      RelationName,
      Relation,
      'Insert',
      Query,
      Result
    >,
    'mutationFn'
  >,
) {
  • Using descriptive names like Schema, Table, Relation, Query, and Result makes the code more intuitive. It reduces the cognitive load required to understand what each type represents.
  • New contributors can grasp the code’s functionality more quickly without deciphering what single-letter generics stand for, thus lowering the barrier to entry. (which was my case few weeks ago)
  • While TypeScript conventions accept single-letter generics (T for type, K for key, etc.), these are best suited for simple, well-understood cases. In complex scenarios with multiple generics, descriptive names enhance clarity.
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