Skip to content

feat: change Repository class extend method #10251

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.

Sign up for GitHub

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

Jump to bottom
Closed
sobierajb wants to merge 2 commits into from
Closed

feat: change Repository class extend method #10251

sobierajb wants to merge 2 commits into from

Conversation

@sobierajb
Copy link

@sobierajb sobierajb commented Aug 3, 2023 •
edited
Loading

Description of change

This change allows to forward custom methods from
extended custom repository and return a repository which contains
previous methods and new defined. This allow to chain custom repositories.
Example:

 const customRepo = manager.getRepository(Foo).extend({
	getFooById(id: number){
		return this.findOne({where: {id: id}});
		}
	})

 const customRepoExt = customRepo.extend({
	getFooByName(name: string){
		return this.findOne({where: {name: name});
		}
	})
 
const customRepoExtExt = manager.withRepository(custoRepoExt).extend({
       getFooByData(date: date){
              return this.findOne({where: {date: date})
       }
})

In this case both methods getFooById and getFooByName are availabe in customRepoExt.

Pull-Request Checklist

  • Code is up-to-date with the master branch
  • npm run format to apply prettier formatting
  • npm run test passes with this change
  • This pull request links relevant issues as Fixes #0000
  • There are new or updated unit tests validating the change
  • Documentation has been updated to reflect this change
  • The new commits follow conventions explained in CONTRIBUTING.md

feat: change Repository class extend method
c421ace 
This change allows to forward custom method from
extended custom Repository and return a new one.
This allow to chain custom repositories.
Example:
 const customRepo = manager.getRepository(Foo).extend({
	getFooById(id: number){
		return this.findOne({where: {id: id}});
		}
	})

 const customRepoExt = manager.withRepository(customRepo).extend({
	getFooByName(name: string){
		return this.findOne({where: {name: name});
		}
	})
In this case both methods getFooById and getFooByName are
availabe in customRepoExt custom Repository
@sobierajb sobierajb closed this Aug 3, 2023
@sobierajb sobierajb reopened this Aug 3, 2023
@hunjin-choi
Copy link
Contributor

hunjin-choi commented Aug 4, 2023

Hello! A couple of minutes ago, I do Pull-Request that alike with your's.
My code is different with your's. But, It will be nice if we discuss this issue.
Please review my PR!

@sobierajb
Copy link
Author

sobierajb commented Aug 4, 2023 •
edited
Loading

Hi! I did review and for me personally it does not matter.
Yours solution creates new class based on "this", prototype new methods for this class and return the instance of it. My creates new object based on "this" and merge with all previously defined methods and methods passed in argument.
This conversation can be reduced to the question if Object.prototype is better than Object.assign in this particular case. Personally I do not have opinion on that ;)

@sobierajb sobierajb mentioned this pull request Aug 7, 2023
Merged
7 tasks
@pleerock
Copy link
Member

pleerock commented Jan 2, 2024

Closing this one since its basically the same as #10256

@pleerock pleerock closed this Jan 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sobierajb @hunjin-choi @pleerock