Skip to content

Revisit cloneWith function #739

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

Open
pokey opened this issue Jun 6, 2022 · 2 comments
Open

Revisit cloneWith function #739

pokey opened this issue Jun 6, 2022 · 2 comments
Labels
code quality Improvements to code quality

Comments

@pokey
Copy link
Member

pokey commented Jun 6, 2022

Defined in BaseTarget.

readonly cloneWith = (parameters: CloneWithParameters) => {
const constructor = Object.getPrototypeOf(this).constructor;
return new constructor({
...this.getCloneParameters(),
...parameters,
});
};

It uses any, so programmer can forget clone parameters without Typescript detecting it. Possible solutions:

  • Enable explicit-function-return-type to force programmer to strongly type their clone parameters
  • Move implementation to derived types, maybe with a utility function that they can use?
  • Figure out a way to make it strongly typed in the base class?
@pokey pokey added the code quality Improvements to code quality label Jun 6, 2022
@AndreasArvidsson
Copy link
Member

Since clone parameters now is typed can we close this one or do we still want to the other suggestions?

protected abstract getCloneParameters(): TParameters;

@pokey
Copy link
Member Author

pokey commented Jul 9, 2023

ah right, I think this one was fixed by #1534. @auscompgeek do you agree?

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

No branches or pull requests

2 participants