Cursor is whole entity base64d in Sequelize 4+

[intellix]

I guess due to this: https://github.com/mickhansen/graphql-sequelize/pull/597

Realised that after pulling in latest code, my cursors were huge: W3siaWQiOjksIm5hbWUiOiJEREQiLCJjb21wbGV0ZWQiOmZhbHNlLCJvdGhlckRhdGUiOiIyMDE1LTExLTE3VDAyOjIzOjU1LjAwMFoiLCJjcmVhdGVkQXQiOiIyMDE1LTExLTE3VDAyOjIzOjU1LjAwMFoiLCJ1cGRhdGVkQXQiOiIyMDE4LTA1LTE0VDE3OjAwOjEyLjQ2OFoiLCJ1c2VySWQiOjEsInByb2plY3RJZCI6Mn0sMF0=

After a base64decode, it turns out that it's:

[{"id":9,"name":"DDD","completed":false,"otherDate":"2015-11-17T02:23:55.000Z","createdAt":"2015-11-17T02:23:55.000Z","updatedAt":"2018-05-14T17:00:12.468Z","userId":1,"projectId":2},0]

I guess the offending lines are: https://github.com/mickhansen/graphql-sequelize/blob/9451c67102582b5938f3beea375d32c7f5f89189/src/relay.js#L157-L160

item.constructor is: function () { Instance.apply(this, arguments); } item.constructor.primaryKeyAttribute is undefined item.Model.primaryKeyAttribute is "id"

item.get(undefined) returns:

{"id":9,"name":"DDD","completed":false,"otherDate":"2015-11-17T02:23:55.000Z","createdAt":"2015-11-17T02:23:55.000Z","updatedAt":"2018-05-14T17:00:12.468Z","userId":1,"projectId":2}

[mickhansen]

I wonder how your cursors looked before that change. It needs to be (item.Model ? item.Model : item.constructor).primaryKeyAttribute to support both versions of sequelize i think.

/cc @jedwards1211

[jedwards1211]

@mickhansen so the previous code before the change was:

   let toCursor = function (item, index) {     let toCursor = function (item, index) {
     let id = item.get(item.constructor ? item.constructor.primaryKeyAttribute : item.Model.primaryKeyAttribute);        let id = item.get(item.constructor ? item.constructor.primaryKeyAttribute : item.Model.primaryKeyAttribute);
-    return base64(PREFIX + id + SEPERATOR + index);    +    return base64(JSON.stringify([id, index]));
   };

So I'm guessing the cursors were something like base64("arrayconnection$[object Object]$3")?

[jedwards1211]

@intellix I think you mean that this is happening in Sequelize 3, not Sequelize 4+. When I tested both versions in node, instance.Model was only defined in Sequelize 3, and instance.constructor.primaryKeyAttribute was only defined in Sequelize 4.

[jedwards1211]

Since users could (unwisely) choose to create an attribute named Model, which would trick @mickhansen 's suggested (item.Model ? item.Model : item.constructor).primaryKeyAttribute, I would recommend making sequelize a peerDependency and just doing

import Sequelize from 'sequelize'
import semver from 'semver'

const oldSequelize = semver.lt(Sequelize.version, '4.0.0')

function getModelOfInstance(instance) {
  return oldSequelize ? instance.Model : instance.constructor
}

Or we could do

import {Model} from 'sequelize'

function getModelOfInstance(instance) {
  return instance instanceof Model ? instance.constructor : instance.Model
}

Mick, let me know which you prefer and I'll make a PR.

[mickhansen]

So I'm guessing the cursors were something like base64("arrayconnection$[object Object]$3")?

Sounds about right, so cursors actually never worked properly?

Regarding the solution, i'd prefer feature checks over version checks - so your secnd version looks good to me.

[intellix]

yep, I noticed arrayconnection$[object Object]$3. To dispel theories of 3, this is my package-lock.json:

sequelize-typescript@0.6.3
sequelize@4.28.5
graphql-sequelize@8.1.0

I'm using Sequelize Typescript, which I guess is probably this causing it? https://github.com/RobinBuschmann/sequelize-typescript/blob/master/lib/models/Model.js#L6

[jedwards1211]

@mickhansen well, I think cursors never worked properly for Sequelize 3 / Sequelize Typescript. @intellix sounds like a bug with Sequelize Typescript to me. While that class inherits from Sequelize's Model class, its constructor doesn't inherit the properties on Sequelize's Model's constructor... I don't really get the whole idea of wrapping Sequelize to provide typing, it seems like a bad idea to me. There are already Typescript defs for Sequelize itself that will not suffer from this issue. (In fact, I used those defs as a starting point for making the Flow-typed defs for Sequelize).

[mickhansen]

@jedwards1211 I think it actually worked fine as the id was never used, just the index.

[jedwards1211]

@mickhansen true, though why is it trying to put the id in the cursors anyway if it doesn't use it?

[mickhansen]

@jedwards1211 Probably some legacy reason.