sequelize icon indicating copy to clipboard operation
sequelize copied to clipboard

Can't get scope abstract constructors to typecheck

Open danprince opened this issue 6 years ago • 17 comments

Trying to get scopes working with these typing and running into a confusing type error. Here's a minimal example that seems to fail with the following versions:

import { Model, DataTypes } from "sequelize";
import sequelize from "../database";

class Message extends Model {}

Message.init(
  {
    private: DataTypes.BOOLEAN
  },
  {
    sequelize,
    scopes: {
      private: {
        where: {
          private: true
        }
      }
    }
  }
);

let PrivateMessages = Message.scope("private");

PrivateMessages.findAll({
  where: {}
});
// The 'this' context of type 'typeof Model' is not assignable to method's 'this' of type '(new () => Model) & typeof Model'.
//   Type 'typeof Model' is not assignable to type 'new () => Model'.
//     Cannot assign an abstract constructor type to a non-abstract constructor type.

Similarly to #112, I would expect typeof Model to be new () => Model, so I'm not sure why this intersection type fails.

danprince avatar Aug 04 '17 14:08 danprince

~~https://github.com/types/sequelize/issues/110 probably related.~~

Nevermind for now, different version.

SimonSchick avatar Sep 15 '17 12:09 SimonSchick

Just wanted to check in on this one again. This error is meaning that we still can't use scopes in our codebase. Are there any examples of scope code typechecking correctly or is this just something that wontfix for this combination of versions?

danprince avatar Nov 20 '17 17:11 danprince

This should be resolved with my fix that was merged some time ago.

SimonSchick avatar Nov 20 '17 19:11 SimonSchick

Afraid not @SimonSchick. Just trying out a few commits between your merge and HEAD.

TypeScript 2.3.4, 2.4.0 and 2.4.2:

let users = await User.scope("human").findAll({ where: { team_id: teamId } });

// The 'this' context of type 'typeof Model' is not assignable to method's 'this' of type '(new () => Model) & typeof Model'.
//   Type 'typeof Model' is not assignable to type 'new () => Model'.
//     Cannot assign an abstract constructor type to a non-abstract constructor type.

TypeScript 2.4.0:

error TS2345: Argument of type '{ where: { "slack.name": string | undefined; }; }' is not assignable to parameter of type 'FindOptions | undefined'.
  Type '{ where: { "slack.name": string | undefined; }; }' is not assignable to type 'FindOptions'.
    Types of property 'where' are incompatible.
      Type '{ "slack.name": string | undefined; }' is not assignable to type '(string | number)[] | Where | WhereAttributeHash | AndOperator | OrOperator | undefined'.
        Type '{ "slack.name": string | undefined; }' is not assignable to type 'OrOperator'.
          Property '$or' is missing in type '{ "slack.name": string | undefined; }'.

Why would the FindOptions require an $or clause in the where object? I would have expected it to match WhereAttributeHash from that union.

TypeScript 2.4.2:

team.set("slack.domain", message.event.name);

// error TS2559: Type '"slack.domain"' has no properties in common with type 'Partial<Team>'.

danprince avatar Nov 21 '17 12:11 danprince

TS attempts to match your type to all listed types and will end up telling you only about one mismatch, the interesting bit is here: (string | number)[] | Where | WhereAttributeHash | AndOperator | OrOperator | undefined.

SimonSchick avatar Nov 21 '17 13:11 SimonSchick

@SimonSchick I'd understand that if it was an intersection type, but why would it report an error when one of the types in the union does match?

danprince avatar Nov 21 '17 13:11 danprince

I think it doesn't like the undefined type union, you probably have strict null checks enabled...

You might want to manually assert that the value is not undefined via if or suffix the index with a ! eg. where: { team_id: teamId! } } as only null is allowed in the WhereAttributeHash type.

SimonSchick avatar Nov 21 '17 13:11 SimonSchick

I do have strict null checks enabled, but the error is thrown regardless of which method I try to invoke on User.scope().

image

I've just turned them off and get the exact same compile errors.

danprince avatar Nov 21 '17 13:11 danprince

Wait, is User abstract in your code?

SimonSchick avatar Nov 21 '17 13:11 SimonSchick

Here's a repro: https://gist.github.com/danprince/6dd8566e22c6a82a29cb57f195c2ff0e

danprince avatar Nov 21 '17 13:11 danprince

Ooooh, I see what you mean now, I ran into the same thing.

So here is what you can do:

Add a override for all scopes you have to your model.

Eg.

public static scope(options?: ScopeStringType | ScopeStringType[] | ScopeOptions | WhereAttributeHash): typeof YourModel {
    return <typeof YourModel> super.scope(options);
  }

Also your scope is wrong, should probably be:

scopes: {
  human: {
    where: {
      slack_id: { $ne: 'USLACKBOT' }
    },
  },
},

SimonSchick avatar Nov 21 '17 14:11 SimonSchick

Thanks for helping out with this! Using that override would mean adding that code to all of our models though right?

danprince avatar Nov 21 '17 14:11 danprince

@danprince all models you call .scope static on, yes. The problem here is that the base .scope has the return signature typeof Model whereas Model is abstract.

SimonSchick avatar Nov 21 '17 14:11 SimonSchick

Ah. I see, but then isn't this broken for everyone?

danprince avatar Nov 21 '17 14:11 danprince

Yes and no, I assume @felixfbecker intention was for this to be overridden on a per model basis.

An alternative would be to change to call signature of .scope to something more generic such as:

export interface ModelConstructor {
  new (...args: []): Model;
}
...
abstract class Model {
  ...
  static scope<M extends ModelConstructor>(this: M, options?: string | Array<string> | ScopeOptions | WhereAttributeHash): M;
}

Thoughts @felixfbecker ?

SimonSchick avatar Nov 21 '17 14:11 SimonSchick

Ideally scope should return typeof this or typeof this['constructor'] but I don't think that's possible. It might be possible to use generics as a workaround, although not pretty.

felixfbecker avatar Nov 21 '17 17:11 felixfbecker

The generics workaround I posted works pretty well.

SimonSchick avatar Nov 21 '17 18:11 SimonSchick