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

task(): remove extend, clone #8599

Closed
ShaMan123 opened this issue Jan 14, 2023 · 3 comments · Fixed by #8600
Closed

task(): remove extend, clone #8599

ShaMan123 opened this issue Jan 14, 2023 · 3 comments · Fixed by #8600
Labels

Comments

@ShaMan123
Copy link
Contributor

ShaMan123 commented Jan 14, 2023

clone is a bad, unmanageable and outdated piece of logic.
In case of cyclic objects it will break
We must remove it for good.
No real reason to use it any longer
I suggest exposing an internal cloneDeep:

export const cloneDeep = <T>(object: T): T =>
  JSON.parse(JSON.stringify(object));

This is good since if cloning a FabricObject it will use the toObject method. And for all others is good enough.

@asturur
Copy link
Member

asturur commented Jan 22, 2023

While not being a issue at all, something i have to point out that struck my eyes is that i removed 3 occurrences of the shitty word around this task and related PRs.

We have a code of conduct that was added after i joined but is still here since long time, and while we may agree or not with every paragraph of it, that's what we have and if i didn't use this occasion to remind ourselves to take additional care when expressing our opinions on code or ideas of other people, i would ignore the code of conduct myself too.

While shitty is exactly as bad as any strong statement that i do use as well, like doesn't make sense or typescript is dumb or i hate typescript, and does also fall in normal language we use everyday, to me shitty is more evident and i noticed it.

It just falls in the unprofessional category we cite in the code of conduct and we can just find some alternatives for it.

@ShaMan123
Copy link
Contributor Author

Point taken.
It was merge conflicts that made it reappear

@asturur
Copy link
Member

asturur commented Jan 23, 2023

i factored those out in the count of 3 🤣

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

Successfully merging a pull request may close this issue.

2 participants