sequelize icon indicating copy to clipboard operation
sequelize copied to clipboard

Type safety of hard coded string attributes (and the usage of `fieldName`)

Open GibsDev opened this issue 9 months ago • 0 comments

Issue Creation Checklist

  • [x] I understand that my issue will be automatically closed if I don't fill in the requested information
  • [x] I have read the contribution guidelines

Bug Description

TLDR

<ModelClass>.getAttributes().<someField>.fieldName seems pretty cumbersome to specify a type safe attribute (or include) during a query, but also, fieldName isn't even typesafe because TS complains about it not existing (a bug?).


Generally the point of Sequelize for me and my team is to make sure that our code is secured by a well formed model structure and types (really looking forward to v7 btw). I've had quite a few run ins with keeping tight types in our code base due to this problem.

It seems like the normal way of specifying attributes/properties on a model/table is a bit loose. You basically just pass in string literals of the things that you're interested in. This offers some great flexibility, but it seems like it's at the expense of type safety.

In an ideal world, if I make a change to a model, it would be nice to know all of the things that are effected by this change. Using hard coded strings as a means to reference properties of a model leaves a gap to where I might find my issues at runtime rather than during transpiling.

Reproducible Example

import {
    Sequelize, Op, Model,
    InferAttributes,
    InferCreationAttributes,
    CreationOptional,
    DataTypes,
} from "sequelize";

const sequelize = new Sequelize({
    dialect: 'sqlite',
    storage: ':memory:',
    logging: false
});

export class Tests extends Model<InferAttributes<Tests>, InferCreationAttributes<Tests>> {
    declare id: CreationOptional<number>;
    declare myField: CreationOptional<string | null>;
    declare createdAt: CreationOptional<Date>;
    declare updatedAt: CreationOptional<Date>;

    static initModel(sequelize: Sequelize): typeof Tests {
        return Tests.init({
            id: {
                type: DataTypes.INTEGER,
                autoIncrement: true,
                primaryKey: true
            },
            myField: {
                type: DataTypes.STRING,
                allowNull: true,
                defaultValue: null
            },
            createdAt: {
                type: DataTypes.DATE,
                allowNull: false,
                defaultValue: DataTypes.NOW,
            },
            updatedAt: {
                type: DataTypes.DATE,
                allowNull: false,
                defaultValue: DataTypes.NOW,
            }
        }, {
            sequelize,
            tableName: 'tests',
            schema: 'public',
            underscored: true,
            timestamps: true
        });
    }
}

(async () => {
    Tests.initModel(sequelize);

    // Shows that there is the existence of `myField.fieldName`
    console.log(Tests.getAttributes());

    // Conflicting typescript error: Property 'fieldName' does not exist on type 'ModelAttributeColumnOptions<Model<any, any>>
    // console.log(Tests.getAttributes().myField.fieldName);

    // @ts-ignore (so it can run) <<-- here lies one problem
    console.log(Tests.getAttributes().myField.fieldName) // 'myField'
    console.log(Tests.getAttributes().myField.field) // 'my_field'

    // The rest of this code will break as the table doesn't exist, but the types are the important part here

    // "Standard" (from what I can tell) way of doing things
    const tests_unsafe = await Tests.findAll({
        attributes: [
            'myField'
        ],
        where: {
            myField: {
                [Op.not]: null
            }
        }
    });

    // Can lead to issues like this one at run time
    const tests_also_unsafe = await Tests.findAll({
        attributes: [
            'howIsThisSafe?'
        ],
        where: {
            myField: {
                [Op.not]: null
            }
        }
    });

    // Something similar to what would be nice to have so we can guarantee the type of the specified attribute
    const tests_still_unsafe_due_to_ts_ignore = await Tests.findAll({
        attributes: [
            // @ts-ignore so we can run it
            Tests.getAttributes().myField.fieldName
            // This also feels quite lengthy for what we are trying to accomplish
        ],
        where: {
            myField: {
                [Op.not]: null
            }
        }
    });
})();

Environment

  • Sequelize version: 6.37.7
  • Node.js version: v22.14.0
  • If TypeScript related: TypeScript version: npm list typescript
  • Database & Version: (I don't believe this is applicable)
  • Connector library & Version: (I don't believe this is applicable)

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.
  • [ ] No, I don't have the time, but my company or I are supporting Sequelize through donations on OpenCollective.
  • [x] 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.

GibsDev avatar Apr 16 '25 03:04 GibsDev