ephys/sequelize-todo.md Secret

## sequelize-todo.md

      
    Raw
  

              sequelize-todo.md
            
          
    Big list of potentials issues I want to tackle at some point but I haven't opened a proper issue or PR yet to avoid spamming.
I also have a backlog now: https://github.com/orgs/sequelize/projects/1
RFCs


sequelize/sequelize#13773 - Sequelize.attr (alternative to Sequelize.col)
https://github.com/sequelize/meetings/issues/12 - Improving auto Timestamp fields
https://github.com/sequelize/meetings/issues/14 - Make Associations TypeScript friendly
https://github.com/sequelize/meetings/issues/15 - Decorator-based model definition
https://github.com/sequelize/meetings/issues/16 - Improve support for Public Class Fields
sequelize/sequelize#14259 - Deprecate DataTypes.STRING, add DataTypes.{BINARY,VARCHAR,VARCHAR.BINARY,VARCHAR.N,CHAR.N}
sequelize/sequelize#14295 - Replace DataTypes.DATE with DataTypes.DATETIME.ZONED
sequelize/sequelize#14297 - pre-fill bind parameters in literal()

Unique

Allow arrays as the value to the 'unique' option of a field:
User.init({
  source: { unique: ['source-slug', 'source-id'] },
  sourceSlug: { unique: 'source-slug' },
  sourceId: { unique: 'source-id' },
});
Unique normalization

Automatically convert the 'unique' option from attributes into indexes, so indexes is the source of truth
Declaring Deferrable FKs

In v6, deferrable cannot be used in belongsTo. It must be used in define instead. We should fix this.
const A = sequelize.define('A', {
  BId: {
    type: DataTypes.INTEGER,
    references: {
      deferrable: Deferrable.INITIALLY_IMMEDIATE() // notice the (), due to wrong typings
    }
  }
});
const B = sequelize.define('B');

A.belongsTo(B);
Should be this instead:
const A = sequelize.define('A');
const B = sequelize.define('B');

A.belongsTo(B, {
  foreignKey: { deferrable: Deferrable.INITIALLY_IMMEDIATE },
});
Enable Deferred Constraints

There should be a way to defer constraint checks for part of a transaction. For instance:
sequelize.transaction(async transaction => {
  // ... do actions with constraints checked immediately.
  
  await transaction.withDeferredConstraints(() => {
    // ... do actions with constraints deferred.
  });

  // ... do actions with constraints checked immediately.
})
Or
sequelize.transaction(async transaction => {
  // ... do actions with constraints checked immediately.
  
  await transaction.deferConstraints();
  
  // ... do actions with constraints deferred.

  await transaction.undeferConstraints();

  // ... do actions with constraints checked immediately.
})
Inheritance

The way Attributes are defined in Sequelize does not play nice with inheritance.
Say you have the inheritance tree Model -> BaseModel -> User, and you have a method that accepts model: ModelStatic<Model> as a parameter. Checking whether Model is a subclass of BaseModel will not let you access the attributes as they could be completely different from a parent model to a child one. i.e. the following does not work:
if (isSubclassOfBaseModel(model)) {
  await model.findAll({
    where: {
      // TypeScript thinks this does not exist.
      baseModelAttribute: 1,
    },
  });
}
Replacements

In Sequelize ? is a positional replacement token, and :name is a named replacement token.
This is confusing because most dialects use ? for bind parameters.
Sequelize use postgres-style bind parameters: $1 and $name.
We should stop using ? as the positional replacement token, and instead use :1, to be symmetrical with our bind parameters and remove the confusion.
Prepared statements

Prepared statements are a good way to optimize both the JavaScript and SQL:

The SQL query is only built once, instead of going through the QueryBuilder every time
The SQL is only parsed once by the database

Design 1 (preferred)

// executed only once:

const getUserByName = User.select()
  .where({ name: bind('name') })
  .toPreparedStatement('find-user-by-name');

// or

const getUserByName = User.prepareFindAll('find-user-by-name', {
  where: { name: bind('name') },
});

// later:

const user = await getUserByName({ name: 'Zoe' });
Advantages:

Clear that prepared statements are created once, and that their contents should not be modified between two calls (outside of passing different bind parameters).
Cleanly incorporates into the query builder API

Disadvantages:

Separates the creation of the query from its usage.

Design 2

const user = await User.select()
  .where({ name: bind('name') })
  .findAll({ preparedStatement: 'find-user-by-name' });

const user = await User.findAll({
  where: { name: bind('name') },
  preparedStatement: 'find-user-by-name',
});
Advantages:

The creation of the query is located at the same place it is used
No need for extra methods on Model

Disadvantages:

We're creating a lot of throwaway objects that will be discarded immediatly
Users may think they can put dynamic values outside of bind parameters, so we'll need to compare

bind() method

bind('name') would be equivalent to doing literal('$name'): it creates a named bind parameter. Especially useful in prepared statements.
Type Safety


DataType.INTEGER should throw if typeof value === 'number' && !Number.isSafeInteger(value)
count & other aggregate functions return number. what if the value is not a representable number? Should have an option to return bigint. Should throw if number is not able to represent the value returned by the dialect.

Support for alternative js types

sequelize/sequelize#14296
Bugs


utils/dialect#diaelcts should include 'snowflake' as it supports milliseconds
lib/utils/sequelize-methods: col has a weird behavior when you give more than one parameter
Model.count({ group: ['count'] }) will return a broken result (because count is both the name of the counted column & the name of the aggregate column). Likely an issue in other aggregate functions too. count could return Array<{ group: object, count: number }> instead of Array<{ ...group, count: number }>

Query Builder

Main article: https://gist.github.com/ephys/078ee6a2eb31dc209f0de6cea95316e7
Improving support for literal strings


sequelize/sequelize#14297
sequelize/sequelize#14299

docs


Update docs on RANGE, it is lackluster

Other features


Add support for reviver argument in DataTypes.JSON(B)
Accept TypeScript ENUM in DataTypes.ENUM?
Add an "after close" hook to Transaction, that is called after either commit or rollback
Add an "after rollback" hook to Transaction
Allow choosing between Validators.js & Joi for validation (@sequelize/validator.js & @sequelize/joi?)
Drop support for ContinuationLocalStorage, migrate to built-in AsyncLocalStorage.
Model.bulkSave for saving many models at once
A 'dialect-aware fn': It receives the dialect and adapts its code based on it. Mostly for internal use, to provide things like Sequelize.now - a dialect-specific 'now' sql function.

Must be usable in userland sequelize/sequelize#9142


Model.findInBatch which returns an async iterator with a simple limit/offset pagination
Model.findByCursor which uses stateless cursor pagination
Model.findByCursorInBatch which returns an async iterator with a stateless cursor pagination

Other changes


Allow specifying logQueryParameters per query instead of globally.
use uniqueidentifier for DataTypes.UUID in mssql?
Specifying limit on an include quietly causes the separate option to be turned on. We should throw instead!
Support all Iterables in { [Op.isIn]: new Set() }?
Can we cast values (not columns)? e.g. cast(myUserProvidedValue, 'JSONB')
Shorter syntax to cast columns: where: { 'preferences::jsonb': {} }
Accept undefined in where but ignore the value. So users can easily add/remove SQL conditions based on JS values
I'm not pursuing this feature anymore as I don't want queries to silently break because a value was accidentally undefined. I'm pursuing work on the Query Builder as an alternative.
Model.increment / Model.decrement return [affectedRows, affectedRowCount]. We could simplify to just returning affectedRows. Need investigation.
Document alias max length for each dialect. If a generated alias is too long: throw with a message recommending to use minifyAliases to resolve the issue
offset in query generator is not strict enough. Maybe forbid anything that's not a safe integer, a big int, or a int string?
Deprecate DataTypes.DATE, replaced with DataTypes.DATETIME.ZONED & DataTypes.DATETIME. sequelize/sequelize#13372 (comment)
Generated columns: sequelize/sequelize#12718
Readonly columns: sequelize/sequelize#4603
sequelize/sequelize#11514
rewrite cli as esm & include cosmiconfig

Improve JSON in MSSQL

use nvarchar(max) + CHECK(ISJSON(col)=1)
TypeCheck include.where

User.findOne({
  include: [{
    association: User.associations.group,
    where: {
      // these are not currently type-checked
      test: 'test',
    }
  }],
});

Housekeeping

I'm using this task to list the few code cleaning tasks that still need to be done

Run eslint on JavaScript snippets in markdown files
Implement review changes from last eslint commit sequelize/sequelize#13930 (review)
Limit packages in commitlint to each dialects + 'types'

Rework queryInterface & queryGenerator

queryInterface and queryGenerator should be proper APIs that users can exploit.
This means most of the code in Model methods should be moved to queryInterface, but be written in a way that specifying models is optional, because it needs to work in transactions too.
This is also true for queryGenerator. You should be able to pass raw FindOptions to it, and it normalizes them itself.
Main thread: sequelize/sequelize#2325 (comment)

bindContext should not include any value.
Instead it should be a simple array to know at which position a named bind parameter is. Methods can then use that array to format the bind option correctly before calling queryRaw
QueryGenerator methods should always return { query, namedBindPositions } (instead of { query, bind }). QueryInterface can then sort out mapping named bind to positional bind (including for insert, update, etc).
eslint changes

Feedback from AllAwesome497:

function-paren-newline would look better like this:

@typescript-eslint/quotes would look better with avoidEscape

@typescript-eslint/explicit-member-accessibility 50/50 vote on enforcing 'public' and forbidding 'public' modifier. Either way it should not be mixed. I personnally would forbid it as it doesn't impact the base JS.