supabase-cache-helpers icon indicating copy to clipboard operation
supabase-cache-helpers copied to clipboard

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

Open damien-schneider opened this issue 1 year ago • 1 comments

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.

damien-schneider avatar Nov 21 '24 22:11 damien-schneider

Happy to accepts PRs on this 🫶🏼

psteinroe avatar Apr 23 '25 07:04 psteinroe