Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v4 to v5 | Problem with sequelize structure, Sequelize module methods auto-complete not working #11250

Closed
neilveil opened this issue Jul 30, 2019 · 24 comments
Assignees
Labels
status: wip For issues and PRs. Applied when the PR is not ready yet / when work to close the issue has started. type: typescript For issues and PRs. Things that involve typescript, such as typings and intellisense.

Comments

@neilveil
Copy link

I'm a developer and I've been using sequelize from last few years. MySQL, MSSQL etc are so vast and updating, I understand that it's hard to cover everything in one ORM. You must have resolved a lot of things in v5 but there is a serious issue that I'm facing right now is auto-complete feature in my code editor (VS code). It might not a big thing for you but for me as a developer it's huge because we developers are totally dependent on this small feature which is removed in v5 as now we are extending sequelize models class. And just because of this I don't want to migrate to v5, it's really useless for me.

@sushantdhiman
Copy link
Contributor

I don't know how we can control VS-Code auto complete but perhaps #8616 (comment) could help?

@papb papb changed the title v4 to v5, Not a great move Autocomplete not working in VSCode Jul 30, 2019
@papb
Copy link
Member

papb commented Jul 30, 2019

Duplicate of #11103

@papb papb marked this as a duplicate of #11103 Jul 30, 2019
@papb papb closed this as completed Jul 30, 2019
@neilveil
Copy link
Author

@sushantdhiman @papb You didn't understand..

Assume I have created a model named Users

Earlier I used to get suggestion like Users.create or Users.update, but in v5 Users is class which extends sequelize.models, and with this structure any code editor can't auto-complete

I am not talking about sequelize module auto-complete, it works!

It's not a duplicate issue!!!

@neilveil neilveil changed the title Autocomplete not working in VSCode v4 to v5 | Problem with sequelize structure, Sequelize module methods auto-complete not working Jul 30, 2019
@papb papb marked this as not a duplicate of #11103 Jul 30, 2019
@papb papb reopened this Jul 30, 2019
@papb
Copy link
Member

papb commented Jul 30, 2019

@neilveil Sorry for the confusion. I have reopened it. Do you think an improvement in the typescript typings could solve this issue?

@papb papb added status: awaiting response For issues and PRs. OP must respond (or change something, if it is a PR). Maintainers have no action type: other For issues and PRs. Something that does not fit in any of the other "type:" labels. labels Jul 30, 2019
@sushantdhiman
Copy link
Contributor

@neilveil May be it will be better if you can share some code for which auto complete does not work. I still don't see how sequelize can fix auto-complete issues for an editor

@aat2703
Copy link

aat2703 commented Aug 8, 2019

I'm also experiencing this... I've just upgraded sequelize-typescript v4 to v5 and in v5 sequelize-typescript uses the native sequelize typescript mappings... The issue seems to be that for example the CreateOptions does not take the model as input and therefor does not know the class attributes.

If CreateOptions could receive a model of type - It would work :)

Check on this line:

options?: CreateOptions

@papb
Copy link
Member

papb commented Aug 8, 2019

@aat2703 As @sushantdhiman said, can you show a real example of the autocomplete not working that we can reproduce?

@aat2703
Copy link

aat2703 commented Aug 8, 2019

Inside the fields: [] attributes cannot be auto-completed or recognized by the IDE. The same is the issue for findOne with the where attributes etc...

		Bookings.create(
			event.body.booking,
			{
				fields: [
					'state',
					'type',
					'startDate',
					'endDate',
					'name',
					'phone',
					'email',
					'guests',
					'note',
					'walkIn'
				],
				include: [
					{
						model: Tables,
						as: 'tables',
						association: 'tables'
					},
					{
						model: BookingTableLinks,
						as: 'setTables',
						attributes: [
							'tableID'
						]
					}
				]
			}
		)

@papb
Copy link
Member

papb commented Aug 8, 2019

@aat2703 You mean this?

Autocomplete should work here?

@aat2703
Copy link

aat2703 commented Aug 8, 2019

Webstorm cannot find the declaration? This works with sequelize-typescript v4 interfaces which generally makes much more sense than native sequelize interfaces... IFindOptions takes a generic model as parameter... And therefor can be a lot more helpful for IDE's

tenant

Lets look at the difference

sequelize-typescript declaration:

https://github.com/RobinBuschmann/sequelize-typescript/blob/a05f4fcaa619ab04455ca87a39dacbeb8447a2c1/lib/models/Model.d.ts#L239-L241

sequelize declaration:

sequelize/types/lib/model.d.ts

Lines 1791 to 1795 in 488c048

public static findOne<M extends Model>(
this: { new (): M } & typeof Model,
options?: FindOptions
): Promise<M | null>;
public static findOne<M extends Model>(this: { new (): M } & typeof Model, options: NonNullFindOptions): Promise<M>;

@papb
Copy link
Member

papb commented Aug 8, 2019

@aat2703 Can you guide me on how to make this intellisense work?

@aat2703
Copy link

aat2703 commented Aug 8, 2019

Making FindOptions take a generic type and pass it into the WhereOptions. This would make auto-completion work anywhere. What do you think?

export type WhereOptions<M> = WhereAttributeHash | AndOperator | OrOperator | Literal | Where | M;

public static findOne<M extends Model>( 
   this: { new (): M } & typeof Model, 
   options?: FindOptions<M>
 ): Promise<M | null>; 
 public static findOne<M extends Model>(this: { new (): M } & typeof Model, options: NonNullFindOptions): Promise<M>; 

@papb
Copy link
Member

papb commented Aug 8, 2019

@aat2703 Thanks for the snippet, although I have to apologize because I wasn't clear... I wanted to ask something else...

What I wanted to ask is: is the behavior you want currently achievable somehow? Some specific IDE, some specific version of sequelize, sequelize-typescript, etc?

@aat2703
Copy link

aat2703 commented Aug 8, 2019

Yeah, it works great with sequelize-typescript v0.6.11 & sequelize v4.43.2

Its because sequelize-typescript < 1 implements its own interfaces that works as i've described above. Starting from sequelize-typescript >= 1 and sequelize version 5.x.x it uses native sequelize typescript interfaces

@papb
Copy link
Member

papb commented Aug 8, 2019

@aat2703 ah, I see! Would you be willing to submit a PR?

@papb papb added the type: typescript For issues and PRs. Things that involve typescript, such as typings and intellisense. label Aug 8, 2019
@aat2703
Copy link

aat2703 commented Aug 8, 2019

Yeah, i'm working on it right now actually 👍

@papb papb added status: wip For issues and PRs. Applied when the PR is not ready yet / when work to close the issue has started. and removed status: awaiting response For issues and PRs. OP must respond (or change something, if it is a PR). Maintainers have no action type: other For issues and PRs. Something that does not fit in any of the other "type:" labels. labels Aug 9, 2019
@maidanskyi
Copy link

maidanskyi commented Oct 6, 2019

@papb any update?
is there opened PR? or maybe it's already merged

@papb
Copy link
Member

papb commented Oct 6, 2019

@maidanskyi I don't know, @aat2703 was apparently working on it but I don't know the state of their work

@ThomasEg ThomasEg mentioned this issue Dec 12, 2019
@ThomasEg
Copy link

I've previously used https://github.com/RobinBuschmann/sequelize-typescript to make this happen... and it works nicely... why not look at how it is implemented there and reuse it in the core of sequelize?

We would like to sponsor someone to implement this....

@papb
Copy link
Member

papb commented Jan 16, 2020

Hi @ThomasEg, perhaps @RobinBuschmann himself would have an idea on an easy way to do this?

I am completely out of time currently

@RobinBuschmann
Copy link
Member

Hey, I'm not 100% sure what this issue is exactly about, but type safety and auto completion for model creation is already addressed with this PR #11820

Regarding fields option in CreateOptions: I think it means the column names not necessarily the attribute names of the model. (@papb Let me know if I'm mistaken) Since camel cased column names can be converted to underscored names using underscored option in ModelOptions autocompletion cannot implemented for this.

@papb
Copy link
Member

papb commented Jan 16, 2020

Thanks for chiming in @RobinBuschmann !

Regarding fields option in CreateOptions: I think it means the column names not necessarily the attribute names of the model. (@papb Let me know if I'm mistaken)

This is correct

@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2021

This issue has been automatically marked as stale because it has been open for 7 days without activity. It will be closed if no further activity occurs. If this is still an issue, just leave a comment or remove the "stale" label. 🙂

@github-actions github-actions bot added the stale label Nov 8, 2021
@WikiRik WikiRik removed the stale label Nov 15, 2021
@ephys
Copy link
Member

ephys commented Feb 5, 2022

There have been a lot of improvements to the typescript typings since this issue was first opened.

I think FindOptions.attributes is still pretty loose. Is there anything else that could be improved in our typings?

@ephys ephys self-assigned this Feb 5, 2022
@ephys ephys closed this as completed Apr 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: wip For issues and PRs. Applied when the PR is not ready yet / when work to close the issue has started. type: typescript For issues and PRs. Things that involve typescript, such as typings and intellisense.
Projects
None yet
Development

No branches or pull requests

9 participants