-
Notifications
You must be signed in to change notification settings - Fork 520
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
Update type.d.ts #1356
Update type.d.ts #1356
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use Poly3.plane
internally, but it's not really meant to be exposed to the user.
Question for @z3dev do we want:
- types to reflect the internal usage, so that we can typecheck more thoroughly within the project
- "hide" the internal representations from the user, so that users of the library don't assume it will be there
@FishOrBear Why add the plane to the class? Does this assist in some of you more difficult designs? |
@z3dev I will access it in my program, but it is not always present, so I marked it as an optional parameter. |
@platypii what do you think? This is probably another example of users doing something that is unexpected. The changes are correct. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FishOrBear these are interesting... but I'm a fan of being detailed. So, let's go with these changes.
please note that the API documents will not provide information on the optional planes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am in general a fan of having more type safety. I think having it as optional type means that users can't depend on it being there so we have optionality in the future. 👍
No description provided.