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

_addIssue blocks every, some & check actions rewrite due to a possible type issue #540

Closed
EltonLobo07 opened this issue Apr 21, 2024 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@EltonLobo07
Copy link
Contributor

EltonLobo07 commented Apr 21, 2024

I was trying to rewrite the every action and I ran into this issue.

Issue

issue

FunctionReference<unknown[], TContext> reduces to (...args: unknown[]) => TContext. This creates a problem. Since function parameter types are contravariant (reference: microsoft/TypeScript#18654), the only valid function types that can be assigned are the functions that have unknown or any parameter types (reference: microsoft/TypeScript#24439). This is the type error I get while rewriting the action:

every-impl-issue

Why did I or any other contributor did not run into this issue earlier while writing actions that were merged to rewrite-with-pipe?
This is because any action's reference passed to _addIssue is reduced to (requirement: unknown, message?: unknown) => <ReturnTypeOfTheActionHere> if the action has two or more overloads with generic parameters. Try commenting the first overload of any action maybe bytes and notice _addIssue creates a type error.

The requirements of every, some and check are functions so they can't be reduced to unknown.

My suggestion

Change FunctionReference<unknown[], TContext> to FunctionReference<any[], TContext> and update all of the relevant affected places. Reading the implementation, I noticed it doesn't really care about the types of the parameters of the function reference passed to _addIssue, so it is safe to use any

Your suggestion/guidance?

Do you have a better suggestion? Also, I apologize if this was a false alarm. I am trying to limit the help needed so that the rewrite is as smooth as possible but I will need some guidance for this issue.

Incomplete every implementation so that you can reproduce the issue:

import type { BaseIssue, BaseValidation, ErrorMessage } from '../../types';
import { _addIssue } from '../../utils/index.ts';

/**
 * Every issue type.
 */
export interface EveryIssue<TInput extends unknown[]>
  extends BaseIssue<TInput> {
  /**
   * The issue kind.
   */
  readonly kind: 'validation';
  /**
   * The issue type.
   */
  readonly type: 'every';
  /**
   * The expected input.
   */
  readonly expected: null;
  /**
   * The validation function.
   */
  readonly requirement: (
    element: TInput[number],
    index: number,
    array: TInput[number][]
  ) => boolean;
}

/**
 * Every action type.
 */
export interface EveryAction<
  TInput extends unknown[],
  TMessage extends ErrorMessage<EveryIssue<TInput>> | undefined,
> extends BaseValidation<TInput, TInput, EveryIssue<TInput>> {
  /**
   * The action type.
   */
  readonly type: 'every';
  /**
   * The expected property.
   */
  readonly expects: null;
  /**
   * The validation function.
   */
  readonly requirement: (
    element: TInput[number],
    index: number,
    array: TInput[number][]
  ) => boolean;
  /**
   * The error message.
   */
  readonly message: TMessage;
}

/**
 * Creates an every validation action.
 *
 * @param requirement The validation function.
 *
 * @returns An every action.
 */
export function every<TInput extends unknown[]>(
  requirement: (
    element: TInput[number],
    index: number,
    array: TInput[number][]
  ) => boolean
): EveryAction<TInput, undefined>;

/**
 * Creates an every validation action.
 *
 * @param requirement The validation function.
 * @param message The error message.
 *
 * @returns An every action.
 */
export function every<
  TInput extends unknown[],
  const TMessage extends ErrorMessage<EveryIssue<TInput>> | undefined,
>(
  requirement: (
    element: TInput[number],
    index: number,
    array: TInput[number][]
  ) => boolean,
  message: TMessage
): EveryAction<TInput, TMessage>;

export function every(
  requirement: (element: unknown, index: number, array: unknown[]) => boolean,
  message?: ErrorMessage<EveryIssue<unknown[]>>
): EveryAction<unknown[], ErrorMessage<EveryIssue<unknown[]>> | undefined> {
  return {
    kind: 'validation',
    type: 'every',
    async: false,
    expects: null,
    message,
    requirement,
    _run(dataset, config) {
      if (dataset.typed && !dataset.value.every(this.requirement)) {
        _addIssue(this, every, 'input', dataset, config);
      }
      return dataset;
    },
  };
}
@fabian-hiller
Copy link
Owner

Thank you for creating this issue and the detailed context. I am currently refactoring part of the rewrite. This will include a fix for this problem. I plan to add this change to the rewrite-with-pipe branch in the next few hours.

@fabian-hiller fabian-hiller self-assigned this Apr 21, 2024
@fabian-hiller fabian-hiller added the bug Something isn't working label Apr 21, 2024
@fabian-hiller
Copy link
Owner

Done. Reach out if you have any questions about my changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants