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

Add two new primitives for MST #2052

Merged
merged 6 commits into from
Aug 10, 2023
Merged

Add two new primitives for MST #2052

merged 6 commits into from
Aug 10, 2023

Conversation

chakrihacker
Copy link
Collaborator

Added:

  • A float type, which would restrict against NaN.
  • A finite type, which would restrict against NaN and infinities

closes #1960

Added:
- A float type, which would restrict against NaN.
- A finite type, which would restrict against NaN and infinities
Copy link
Collaborator

@coolsoftwaretyler coolsoftwaretyler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @chakravarthi-bb - thank you for this! Looks great.

Would you mind also updating the docs site with these changes? The API change that got auto generated in your PR is useful, but I think we also need to update the Primitive types section in docs/overview/types.md.

Let me know once you've done that, I'll take another look, and we should be good to go. I think we'll get this in the next minor release, but I'm not exactly sure when that'll be (hopefully soon).

@coolsoftwaretyler
Copy link
Collaborator

Hey @chakrihacker - just a heads up that I merged an older PR adding types.lazy - #1722. You'll have some merge conflicts in this branch, I believe. I'm hoping to release an (alpha) minor build sometime next week. If you get around to my feedback + rebasing in the next few days, this PR should make it in there. Otherwise, we can always run a new minor build later on.

Thanks!

@chakrihacker
Copy link
Collaborator Author

I will update this week

@chakrihacker
Copy link
Collaborator Author

@coolsoftwaretyler Can you review please?

Copy link
Collaborator

@coolsoftwaretyler coolsoftwaretyler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chakrihacker - thanks for the good work and the rebase!

I'm going to make two small commits against this PR to fix some docs, but other than that, I'm going to approve it, merge it, and get it out for the alpha build of the next minor release (hopefully tonight).

@@ -116,6 +118,44 @@ export const integer: ISimpleType<number> = new CoreType<number, number, number>
(v) => isInteger(v)
)

/**
* `types.float` - Creates a type that can only contain an float value.
* This type is used for float values by default
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* This type is used for float values by default

nitpick: @chakrihacker - this is not technically true, I don't think we infer types.float for floating point numbers. I think this comment is true of types.number, based on how getPrimitiveFactoryFromValue behaves, which is what we call in toPropertiesObject when doing type inference.

I'm going to make this edit since it's small and really more of a nitpick than anything.


/**
* `types.finite` - Creates a type that can only contain an finite value.
* This type is used for finite values by default
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: @chakrihacker - this is not technically true, I don't think we infer types.finite with this change. I think this comment is true of types.number, based on how getPrimitiveFactoryFromValue behaves, which is what we call in toPropertiesObject when doing type inference.

I'm going to make this edit since it's small and really more of a nitpick than anything.

@coolsoftwaretyler coolsoftwaretyler merged commit b8bce35 into mobxjs:master Aug 10, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add float (or finite) type
3 participants