Remove Dynamoose interfaces

[MickL]

Could you please remove all interfaces related to Dynamoose?

It now has its own type definitions and it could happen that the IDE imports typings from nestjs-dynamoose by mistake and those type definitions are deprecated.

I have used: 0.2.0-beta.2

Also readme should be updated: Model should not be imported from nestjs-dynamoose but from dynamoose

[MickL]

Or maybe Model should be kept in nestjs-dynamoose if it is/behaves differently but then it should extend interfaces from dynamoose if possible. The best would be if one does not have to keep nestjs-dynamoose interfaces manually in sync with dynamoose interfaces.

E.g. in your QueryInterface the functions descending and ascending do not exist in Dynamoose 2.x. But there will be a new sort function: https://github.com/dynamoose/dynamoose/pull/860

[hardyscc]

the current Model interface from dynamoose is still broken, therefore I need to maintain a copy here, however I will review the interfaces and match the new v2 api as much as possible.

[MickL]

Could you explain what you mean by broken?

I will review the interfaces and match the new v2 api as much as possible

Thanks for your efforts!

Even tho he released 2.x it seems to be still in beta and changes and fixes are currently common. I am kind of afraid that changes made are not reflected in this project if there is a copy. It could be even in years that Dynamoose gets updated but this package is no more maintained so I hope one day this package will not redefine the interfaces of Dynamoose. I leave this issue open until then.

[hardyscc]

Yes eventually all interfaces should be under dynamoose, I will make this happen :)

[hardyscc]

@MickL I just released 0.2.0-beta.3 would you mind to have a test. thanks :)

[hardyscc]

Could you explain what you mean by broken?

because of

https://github.com/dynamoose/dynamoose/blob/5ae249b680e758bdff9cd29c3fd4d84e0ad5172c/lib/DocumentRetriever.ts#L117

the query cannot be chained like that

  this.model.query('targetId')
  .eq(targetId)
  .exec();

[pfulop]

Hi @hardyscc , I recently did a PR https://github.com/dynamoose/dynamoose/pull/890 that is fixing some of the type definitions in dynamoose. Mick mentioned that this project had some custom interface that eventually should be moved to the dynamoose project and that I should team up with you. What's the strategy here.

[hardyscc]

Hi @pfulop, I also facing the same problem, As you know now dynamoose is using a Condition class which shared with scan() and query(), however I cannot find a way to type it correctly when do a query chain like this

  this.model.query('targetId')
  .eq(targetId)
  .exec();

Therefore I just create a type file to overcome it in this project. please see https://github.com/hardyscc/nestjs-dynamoose/blob/master/lib/interfaces/model.interface.ts

[pfulop]

So just to double-check, the issue is that exec in dynamoose doesn't exist on Condition.

So you created ScanInterface and QueryInterface just to be able to call exec on the last condition. If that's the case I can fix it today.

[hardyscc]

@pfulop yes correct :)

[pfulop]

@hardyscc please see actual changes here https://github.com/dynamoose/dynamoose/pull/890/commits/92d1c2019a09d1e771b6c7231d329ce408664532

I took some liberties and removed one of the TODOs concerning duplicity.

So with code such as this:

 const queryResult = Stat.query('targetId').eq('targetId').exec();
 const scanREsult = Stat.scan('targetId').eq('targetId').exec();`
 the infered types are

const queryResult: Promise<QueryResponse<dynamoose.Document[]>>
const scanREsult: Promise<ScanResponse<dynamoose.Document[]>>

I also think I can get more granular and work it in a way where the exec result won't be a generic document but rather a document for a specific model. But first I wanted to make sure that you're ok with the changes.

[hardyscc]

@pfulop Yes this is definitely the right direction, thanks for your great help!

Furthermore is it possible to support additional generic type for model's key like this?

export interface UserKey {
  id: string;
}

export interface User extends UserKey {
  name: string;
  email?: string;
}

@Injectable()
export class UserService {
  constructor(
    @InjectModel('User')
    private userModel: Model<User, UserKey>,
  ) {}

  update(key: UserKey, user: Partial<User>) {
    return this.userModel.update(key, user);
  }
}

[pfulop]

I think this shall do

https://github.com/dynamoose/dynamoose/pull/890/commits/a94b1a8b4a2b8fa27f34ee09f492a99d8e6999da

[hardyscc]

@pfulop that is wonderful! I am looking forward for this PR. ^^

[hardyscc]

Just remind myself here, still cannot close this issue yet, need generic type support for Key.