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

Add a rule to enforce TypeORM's Relation<...>-wrapper #16

Closed
wants to merge 4 commits into from

Conversation

lmeysel
Copy link

@lmeysel lmeysel commented Oct 18, 2024

When migrating to ESM this wrapper is required to avoid dependency conflicts. The new rule enforce-relation-wrapper avoids to forget this wrapper and makes migration to ESM on an existing codebase pretty easy (since are not updated by hand)

See: https://typeorm.io/#relations-in-esm-projects

Additionally I added support for this wrapper also to the existing rules, that they do not raise issues if references are wrapped as e.g. Relation<Other>

Happy to discuss this!

@daniel7grant
Copy link
Owner

Thank you for your contribution!

My question is, could this rule be part of enforce-relation-types? One could define "enforce-relation-types": ["error", {"relationWrapper": true}] and it would handle both the correct inner types, and require the wrapper to be around it. I didn't have time to look at the code extensively, can you tell me why you opted for the separate rule?

@lmeysel
Copy link
Author

lmeysel commented Oct 21, 2024

TBH I did not think about this. I wrote the rule a while ago and now decided to publish it, found your repository and here is the PR... :)

I think it could be part of the enforce-relation-types rule. However I made some more advanced attempts to respect existing imports and aliases if applicable and would still proceed to maintain. If I merge the rules I would also apply these to existing rules.: E.g. when import { OneToMany as OTM } from 'typeorm', this alias is maintained and considered (although this kind of "type-checking" is not actually part of eslint, but gave a bit more stability in my particular case). This would also affect the existing rules.

However, if you are fine with that I would merge these rules and update the PR as soon as I find the time

@daniel7grant
Copy link
Owner

I looked at your code, and decided to combine the rule with the already existing enforce-relation-types. Actually it was much simpler than expected, since I already had another wrapper type for lazy relations (Promise<T>), so I just needed to change that a little. I also added support for forcing every relation to Relation<T>, I put together these in #18. Feel free to take a look at it, and I will put out a new version in the following days.

As far as the import aliases go, I eventually decided against it. I don't really know any ESLint rule that looks at or changes imports and even though it could be a cool feature, it would be very niche usage. I hope you don't mind it, I just didn't want to add the extra maintenance overhead. Thanks for your contribution anyways.

@daniel7grant
Copy link
Owner

I merged this in #18, thank you for raising this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants