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

Cannot use WhereOptions, specifically WhereAttributeHash with extended generics #15179

Open
1 of 4 tasks
JamieREvans opened this issue Oct 24, 2022 · 4 comments
Open
1 of 4 tasks
Labels
type: typescript For issues and PRs. Things that involve typescript, such as typings and intellisense.

Comments

@JamieREvans
Copy link

Bug Description

Sequelize v6.23.0

I cannot use WhereOptions, more specifically WhereAttributeHash with generics. If I use a non-generic type, I do not have any issues. However, if I use a generic type which extends the type that worked without issues, I get errors when trying to create the hash.

Reproducible Example

import {
	Model,
	WhereAttributeHash,
	WhereAttributeHashValue
} from 'sequelize';

class BaseModel extends Model {
	id: string;
}

class ExampleClass<M extends BaseModel> {
	public example() {
		// Both fine
		const thing: M['id'] = 'abc123';
		const other: BaseModel['id'] = 'abc123';

		/**
		 * Type 'number' is not assignable to type 'M["id"]'.
		 *   Type 'number' is not assignable to type 'string'.
		 */
		const fails: M['id'] = 123;
		/**
		 * Type 'number' is not assignable to type 'string'.
		 */
		const alsoFails: BaseModel['id'] = 123;

		// Fine
		let baseModelId: WhereAttributeHashValue<BaseModel['id']> = thing;
		/**
		 * Type 'M["id"]' is not assignable to type 'WhereAttributeHashValue<M["id"]>'.
		 *   Type 'string' is not assignable to type 'WhereAttributeHashValue<M["id"]>'.
		 *     Type 'M["id"]' is not assignable to type 'WhereAttributeHash<any>'.
		 *       Type 'string' is not assignable to type 'WhereAttributeHash<any>'.
		 *         Type 'string' is not assignable to type '{ [x: string]: any; }'.
		 *           Type 'M["id"]' is not assignable to type '{ [x: string]: any; }'.
		 *             Type 'string' is not assignable to type '{ [x: string]: any; }'.
		 */
		let genericModelId: WhereAttributeHashValue<M['id']> = thing;

		// Fine
		baseModelId = other;
		/**
		 * Type 'M["id"]' is not assignable to type 'WhereAttributeHashValue<M["id"]>'.
		 *   Type 'string' is not assignable to type 'WhereAttributeHashValue<M["id"]>'.
		 *     Type 'M["id"]' is not assignable to type 'WhereAttributeHash<any>'.
		 *       Type 'string' is not assignable to type 'WhereAttributeHash<any>'.
		 *         Type 'string' is not assignable to type '{ [x: string]: any; }'.
		 *           Type 'M["id"]' is not assignable to type '{ [x: string]: any; }'.
		 *             Type 'string' is not assignable to type '{ [x: string]: any; }'.
		 */
		genericModelId = other;

		// Fine
		const validWhere: WhereAttributeHash<BaseModel> = {
			id: 'abc123'
		};
		/**
		 * Type '{ id: "abc123"; }' is not assignable to type 'WhereAttributeHash<M>'.
		 *   Type '{ id: "abc123"; }' is not assignable to type '{ [AttributeName in keyof M as AttributeName extends string ? AttributeName | `$${AttributeName}$` : never]?: WhereAttributeHashValue<M[AttributeName]> | undefined; }'.
		 */
		const failingWhere: WhereAttributeHash<M> = {
			id: 'abc123'
		};
	}
}

Following the type path, it should:

  1. Follow the first option in WhereAttributeHash
    [AttributeName in keyof TAttributes as AttributeName extends string ? AttributeName | `$${AttributeName}$` : never]?: WhereAttributeHashValue<TAttributes[AttributeName]>;
  2. Follow the first option in WhereAttributeHashValue (second option in the generic expression)
    AllowNotOrAndRecursive<WhereOperators<AttributeType>[typeof Op.eq]>
  3. Which is equal to:
    AllowAnyAll<OperatorValues<AttributeType>>
  4. Which evaluates OperatorValues to:
type StaticValues<Type> =
  Type extends Range<infer RangeType> ? [lower: RangeType | RangePart<RangeType>, higher: RangeType | RangePart<RangeType>]
  : Type extends any[] ? { readonly [Key in keyof Type]: StaticValues<Type[Key]>}
  : Type extends null ? Type | WhereSerializableValue | null
  : Type | WhereSerializableValue;
  1. Which should evaluate on the last check where Type is string, resolving back up to AllowAnyAll which would resolve string, then so on and so forth back up to the top.

What do you expect to happen?

What is actually happening?

Environment

  • Sequelize version:
    • └── sequelize@6.23.0
  • Node.js version:
    • v14.18.2
  • If TypeScript related: TypeScript version:
    • └── typescript@4.8.3
  • Database & Version:
    • N/A
  • Connector library & Version:
    • N/A

Would you be willing to resolve this issue by submitting a Pull Request?

  • Yes, I have the time and I know how to start.
  • Yes, I have the time but I will need guidance. (I mostly just don't understand the issue well enough to solve it)
  • No, I don't have the time, but my company or I are supporting Sequelize through donations on OpenCollective.
  • No, I don't have the time, and I understand that I will need to wait until someone from the community or maintainers is interested in resolving my issue.

Indicate your interest in the resolution of this issue by adding the 👍 reaction. Comments such as "+1" will be removed.

@WikiRik WikiRik added the type: typescript For issues and PRs. Things that involve typescript, such as typings and intellisense. label Nov 7, 2022
@steveetm
Copy link

Any solution/workaround? I'm trying to create a class to generate where, to add various conditions dynamically. I would like to do it in a typesafe manner, but struggling with that, probably because the same reason.

@ephys
Copy link
Member

ephys commented Mar 2, 2023

Dynamic typing of attributes when the model itself is a generic is not possible with the current design. It's something I plan on fixing in a future major release

@Kagu-chan
Copy link

Hi @ephys
I'm not sure if i understand you correctly.
We're trying to update from 6.18 to current version (I think we used fixed version back then due to typing issues and no time, then forgot about it...)

Is the following an issue resulting from this?

  • an abstract class implementing any Model type
  • WhereOptions and UpdateOptions behave not properly with this update
import {
  // [...]
  UpdateOptions,
  WhereOptions,
} from 'sequelize';
import { Model } from 'sequelize-typescript';
// Model from sequelize-typescript adds the id property for us as `number | any`

export abstract class AbstractRepository<TModel extends Model> {
  // [...]

  public async updateByPk(data: Partial<TModel>, id: number): Promise<TModel> {
/*
Type '{ id: number; }' is not assignable to type 'WhereOptions<TModel>'.
  Types of property 'id' are incompatible.
    Type 'number' is not assignable to type 'WhereAttributeHashValue<TModel["id"]>'.
*/
    const [result] = this.update(data, { where: { id } }); // the old code failing with the message above

    // With this we can fix it, but only sometimes... depending on context
    // const [result] = this.update(data, {
    //   where: { id: id as unknown as WhereAttributeHashValue<TModel['id']> },
    // });

    // The new expected where condition is the property nested in itself
    // const [result] = this.update(data, { where: { id: { id } } });

    return result;
  }

  public async update(data: Partial<TModel>, options?: UpdateOptions<TModel>): Promise<TModel[]> {
    // [...]
  }

  // Here WhereOptions behave the same way as UpdateOptions
  public async checkForPossibleDuplicates(
    data: Partial<TModel> | Partial<TModel>[],
    where?: WhereOptions<TModel>,
  ): Promise<void> {
    // [...]
  }

/*
Old usage:
          await this._repo.checkForPossibleDuplicates(dto, { id: command.id });

New? usage
          await this._repo.checkForPossibleDuplicates(dto, { id: { id: command.id } });
*/
}

I've read through PR 14022 and PR 14368 to find out whats happening there, in the latter this exact issue is actually mentioned, but not resolved.
I did try to find other issues with this, but try searching for where in the github search O.o ...

I'd be happy to open a proper issue if this has nothing to do with the above

@atrick-speedline
Copy link
Contributor

This is broken for us too. It's annoying because we're trying to upgrade in order to patch some sql injections.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: typescript For issues and PRs. Things that involve typescript, such as typings and intellisense.
Projects
None yet
Development

No branches or pull requests

6 participants