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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tag BoudingBox.getPoints() as a readonly array return #3020

Open
eonarheim opened this issue Apr 8, 2024 Discussed in #3007 · 2 comments
Open

Tag BoudingBox.getPoints() as a readonly array return #3020

eonarheim opened this issue Apr 8, 2024 Discussed in #3007 · 2 comments
Labels
bug This issue describes undesirable, incorrect, or unexpected behavior

Comments

@eonarheim
Copy link
Member

Discussed in #3007

Originally posted by ikudrickiy April 6, 2024
The question may seem stupid. But I'm not afraid of those 馃槈

This public function returns the same array referrence for multiple function calls. I can change the array in one place and it will be changed for all the calls. At the same time, if we assume cloning I don't understand the need for a caching mechanism. It looks like a burden in this case. And if there's no caching, cloning seems like a burden.

We can solve this problem on a typescript level turning the function return into readonly array (see this).

@eonarheim eonarheim added the bug This issue describes undesirable, incorrect, or unexpected behavior label Apr 8, 2024
@mattjennings
Copy link
Contributor

there are plenty of places in the codebase that do this, no? would we want to do this everywhere...? i feel like that's going to be difficult to stay consistent on.

(i personally don't see this as much of a problem... adding readonly will help the end user but i think this is just something you need to be aware of when using javascript - the ramifications of mutating received arrays)

@ikudrickiy
Copy link
Contributor

@mattjennings there is nothing wrong with gradually adding a readonly tag in all the necessary places. While we don't have such a functionality in javascript, it is present in typescript.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue describes undesirable, incorrect, or unexpected behavior
Projects
None yet
Development

No branches or pull requests

3 participants