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

fix(intl): expose Intl.Common types #197

Merged
merged 3 commits into from
Feb 24, 2024
Merged

Conversation

glennsl
Copy link
Contributor

@glennsl glennsl commented Feb 24, 2024

The Intl.Common types don't seem to be exposed. And while they technically don't need to be, since they're structural, it's useful to be able to reference them for derived APIs (as I have in a project of mine).

It might be more consistent to expose the types directly in relevant modules though, instead of the module as a whole, but this is the minimal change needed.

@glennsl
Copy link
Contributor Author

glennsl commented Feb 24, 2024

Also adds JS artifacts for recently added tests. Not sure if they need to be tracked, but we should either add them or explicitly ignore them to prevent them from being accidentally added in other PRs.

@zth
Copy link
Collaborator

zth commented Feb 24, 2024

@glennsl perfect, thank you for cleaning up as well! Would you like a 1.1.1 with this so you can use this in your project right away? Easily done if so.

@zth zth merged commit 43daa8c into rescript-lang:main Feb 24, 2024
6 checks passed
@glennsl glennsl deleted the patch-2 branch February 24, 2024 15:29
@glennsl
Copy link
Contributor Author

glennsl commented Feb 24, 2024

Thanks, but no need for my sake. I found the zeroTo20 and zeroTo21 types a bit too restrictive amyway, so I just continued to use ints and sprinkled some magic on them instead.

illusionalsagacity pushed a commit to illusionalsagacity/rescript-core that referenced this pull request Jun 19, 2024
* fix(intl): expose Intl.Common types

* docs: update change log

* chore: add js artifacts for tests
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.

2 participants