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

Use BuckleScript's native namespacing feature. #182

Merged
merged 1 commit into from
Jun 21, 2020

Conversation

parkerziegler
Copy link
Contributor

This PR adds support for using BuckleScript's native "namespace: true" feature. Rather than all of our module names being Urql* and then renamed in ReasonUrql.re, we just use BuckleScript's native "namespace: true". This is great – our codebase is easier to traverse and module references are more succint. This to address feedback from #170. Would love to get any feedback or opinions from folks before merging. For the most part, the public API doesn't really change. The one piece that does is UrqlTypes is now just Types. So if you do the following:

open ReasonUrql;

and you have a local Types module, you'd get a conflict. Not a huge deal though, because you can make the (wiser) choice to not globally open ReasonUrql and just reference it as needed. If approved, I'll update our code samples to show that pattern a bit more often.

Copy link

@kitten kitten left a comment

Choose a reason for hiding this comment

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

Absolutely amazing work! Must’ve taken a while to rename all of this 😅 I didn’t think it’d work out this smoothly with a couple of renames 🤩🙌

@parkerziegler
Copy link
Contributor Author

Yeah fortunately there's not too much in repo module referencing – mostly just modules referencing UrqlTypes and needing to get upgraded to Types. But yeah, much smoother than anticipated!

@parkerziegler parkerziegler merged commit 662191d into master Jun 21, 2020
@parkerziegler parkerziegler deleted the task/namespacing branch June 21, 2020 23:31
tatchi pushed a commit to tatchi/reason-urql that referenced this pull request Aug 25, 2020
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