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 code in validations-and-constraints.md #794

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jthemphill
Copy link

@jthemphill jthemphill commented Mar 13, 2025

Two kinds of errors here:

  1. Sequelize.STRING and Sequelize.INTEGER don't exist. These should be DataTypes.STRING and DataTypes.INTEGER instead.
  2. TypeScript assumes that this corresponds to the ModelValidateOptions that the validation method is defined in.

In a TypeScript file, we would write

User.init({
  // ...,
  validate: {
    customValidator(this: User, value: string | null) {
      // ...
    }
  }
})

But your examples are in JS, so I'm writing a JSDoc equivalent. While I wouldn't usually force types upon people who choose to write untyped code, this is a special case. Without this, the people who do choose to use types will otherwise see the wrong type here.

Two kinds of errors here:

1. `Sequelize.STRING` and `Sequelize.INTEGER` don't exist. These should be `DataTypes.STRING` and `DataTypes.INTEGER` instead.
2. TypeScript assumes that `this` corresponds to the `ModelValidateOptions` that the validation method is defined in.

In a TypeScript file, we would write

```ts
User.init({
  ...,
  validate: {
    customValidator(this: User, value: string | null) {
      // ...
    }
  }
```

But your examples are in JS, so I'm writing a JSDoc equivalent. While I wouldn't usually force types upon people who choose to write untyped code, this is a special case. Without this, the people who *do* choose to use types will otherwise see the *wrong* type here.
@jthemphill jthemphill requested a review from a team as a code owner March 13, 2025 23:14
@jthemphill jthemphill requested review from sdepold and ephys March 13, 2025 23:14
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

Successfully merging this pull request may close these issues.

1 participant