sequelize / sequelize-typescript

Decorators and some other features for sequelize
MIT License
2.79k stars 280 forks source link

'order' option can't be a string #82

Closed bucketheadv closed 7 years ago

bucketheadv commented 7 years ago

Hello,

when I using the order option,

AppUser.find({ order: 'id DESC' }) // will throw error
AppUser.find({ order: [['id', 'DESC']] }) // This works well

When I use order as a string, the error below will be thrown:

Error: Order must be type of array or instance of a valid sequelize method.
      at Object.getQueryOrders (node_modules/sequelize/lib/dialects/abstract/query-generator.js:1692:13)
      at Object.selectQuery (node_modules/sequelize/lib/dialects/abstract/query-generator.js:1161:27)
      at QueryInterface.select (node_modules/sequelize/lib/query-interface.js:672:27)
      at Promise.try.then.then.then (node_modules/sequelize/lib/model.js:1538:34)
      at tryCatcher (node_modules/bluebird/js/release/util.js:16:23)
      at Promise._settlePromiseFromHandler (node_modules/bluebird/js/release/promise.js:512:31)
      at Promise._settlePromise (node_modules/bluebird/js/release/promise.js:569:18)
      at Promise._settlePromise0 (node_modules/bluebird/js/release/promise.js:614:10)
      at Promise._settlePromises (node_modules/bluebird/js/release/promise.js:693:18)
      at Async._drainQueue (node_modules/bluebird/js/release/async.js:133:16)
      at Async._drainQueues (node_modules/bluebird/js/release/async.js:143:10)
      at Immediate.Async.drainQueues (node_modules/bluebird/js/release/async.js:17:14)

The order's definition is:

order?: string | col | literal | Array<string | number | typeof Model | {
        model: typeof Model;
        as?: string;
    }> | Array<string | col | literal | Array<string | number | typeof Model | {
        model: typeof Model;
        as?: string;
    }>>;

So, I think it's a bug.

RobinBuschmann commented 7 years ago

Thanks for reporting. The original sequelize typings seem to have the same issue: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/sequelize/index.d.ts#L3243

Additionally I cannot find any possibilities to use strings in order from the sequelize docs.

Which sequelize version are you using?

chanlito commented 7 years ago

I think only array should be allowed, because I believed I've read in the Sequelize docs that order shouldn't be with strings. although it works

RobinBuschmann commented 7 years ago

@BruceHem You mean it is working with strings? In which sequelize version?

chanlito commented 7 years ago

Sequelize v3 order can be used with strings last time I checked.

bucketheadv commented 7 years ago

Yes, but v4 can't any more.

RobinBuschmann commented 7 years ago

@bucket-sven @BruceHem Thanks.

If it is working on v3, I would suggest to not change the typings for now. When dropping v3 support some day, this issue should be fixed as well.

bucketheadv commented 7 years ago

@RobinBuschmann OK.

andineck commented 6 years ago

I just ran into this problem as well:

Order must be type of array or instance of a valid sequelize method. at Object.getQueryOrders

with sequelize v4.19.0

The docs still has the "string example": http://docs.sequelizejs.com/manual/tutorial/models-usage.html Project.findAll({order: 'title DESC'})

I think this is misleading.

freeeve commented 6 years ago

@andineck I ran into it, too.

It looks like the proper way should be: Project.findAll({order: [sequelize.literal('title DESC')]})

markcellus commented 5 years ago

FWIW, I ran into this issue too. Docs should be updated to remove the Project.findAll({order: 'title DESC'}) example that @andineck mentioned earlier.

mvelebit commented 5 years ago

Still not removed from the docs, +1