Enable association compatibility with Sequelize

[lightpriest]

When migrating to sequelize-typescript, if model A has two associations to model B, one without an alias configuration and one which is aliased, the migration is not possible at all.

ModelA.hasMany(ModelB)
ModelA.hasMany(ModelB, { as: "secondB" })

ModelA.findAll({ include: [ModelB] });

When configured using sequelize, it attempts to find the intended association. In our case, it's obvious we want ModelB without the alias. This is true especially if you have a lot of legacy code and you've added a second association. You don't expect the existing code to break having to add the alias variant of the include on every query.

When configured using sequelize-typescript, its HasMany and other association decorators always add an alias (as: ..) configuration to the options object. This later causes the above query findAll to fail with a message saying the alias cannot be inferred.

This PR modifies a few different places and allows to opt-out of this behavior.

1. The decorators themselves will not include the alias if explicitly set to undefined.
2. The inferrence service matches sequelize's behavior and looks for non-aliased associations.

For number 1, the change in behavior should not really be noticed, unless someone used undefined explicitly for some reason. And even then, it should only be noticed if the model name differs from the propery name.

Normally passing around undefined should be seen as a code-smell, but for the sake of backwards compatibility and the fact that this allows easier migration of large codebases, I see this as negligble.

Added relevant tests and updated the README.

A general note

We patched this internally for our implementation, and running like this for the past 1.5 years in production. With a sequelize code base of >100 models, which spans a few years with all the edge-cases you could possibly think of, including polymorphism associations. I can safely say this is battle tested.

On top of that, our implemented fix was a little bit different, not including aliases at all unless specified, and always using properyName for the value. Generally speaking, allowing the developer to specify an alias which differs from the properyName would yield unexpected results. This is something that needs to be handled in the repo but on a different track, since it adds backwards incompatibility if applied.

[lightpriest]

If you'd like to better understand which pattern this fixes, here's a complete example with sequelize and sequelize-typescript. The expected behavior is that they should both work, but the typescript version fails.

// Simply using sequelize
import { Sequelize, DataTypes, Model } from 'sequelize';

const sequelize = new Sequelize('sqlite::memory:');

class User extends Model {}
class Comment extends Model {}

User.init({
  id: {
    type: DataTypes.INTEGER,
    primaryKey: true,
    autoIncrement: true,
  }
}, { sequelize });

Comment.init( {
  id: {
    type: DataTypes.INTEGER,
    primaryKey: true,
    autoIncrement: true,
  }
}, { sequelize });

User.hasMany(Comment, { as: undefined });
User.hasMany(Comment, {
  as: 'archivedComments'
});

(async function run() {
  await sequelize.sync();
  console.log(await User.findAll({ include: [Comment, 'archivedComments'] }));
})();

// Outputs an empty array

// Using sequelize-typescript
import { HasMany, Model, Table, Sequelize, ForeignKey, Column } from 'sequelize-typescript';

const db = new Sequelize('sqlite::memory:');

@Table({ modelName: 'user' })
class User extends Model {
  @HasMany(() => Comment)
  comments: Comment[];

  @HasMany(() => Comment, {
    as: 'archivedComments'
  })
  archivedComments: Comment[];
}

@Table({ modelName: 'comment' })
class Comment extends Model {
  @ForeignKey(() => User)
  @Column
  userId: number
}

db.addModels([User, Comment]);

(async function run() {
  await db.sync();
  console.log(await User.findOne({ include: [Comment, 'archivedComments'] }));
})();

// Raises an exception:
// (node:14365) UnhandledPromiseRejectionWarning: Error: Alias cannot be inferred: "user" has multiple relations with "comment"
//     at inferAliasForInclude (.../sequelize-typescript-fix/node_modules/sequelize-typescript/dist/associations/alias-inference/alias-inference-service.js:45:23)
//     at .../sequelize-typescript-fix/node_modules/sequelize-typescript/dist/associations/alias-inference/alias-inference-service.js:23:19
//     at Array.map (<anonymous>)
//     at inferAlias (.../sequelize-typescript-fix/node_modules/sequelize-typescript/dist/associations/alias-inference/alias-inference-service.js:22:39)
//     at Function.Model.<computed> [as findAll] (.../sequelize-typescript-fix/node_modules/sequelize-typescript/dist/model/model/model.js:137:74)
//     at .../sequelize-typescript-fix/sequelize-ts.ts:29:26
//     at step (.../sequelize-typescript-fix/sequelize-ts.ts:57:23)
//     at Object.next (.../sequelize-typescript-fix/sequelize-ts.ts:38:53)
//     at fulfilled (.../sequelize-typescript-fix/sequelize-ts.ts:29:58)

[ephys]

The behavior in Sequelize 7 is closer to what Sequelize-TypeScript does than Sequelize 6 (though not strictly equivalent). I'd be interested in your feedback regarding these changes in the upcoming major release: https://sequelize.org/docs/v7/other-topics/upgrade/#associations-names-are-now-unique

[lightpriest]

The behavior in Sequelize 7 is closer to what Sequelize-TypeScript does than Sequelize 6 (though not strictly equivalent). I'd be interested in your feedback...

This has always seemed like a sequelize issue to me and not a sequelize-typescript issue. It's just that sequelize-typescript should probably be compatible to what sequelize is doing. It does seem like sequelize are fixing it properly now, so it probably won't be needed here. Up to you.