Relude_Unsafe shadows our projects Unsafe module which has more stuff in it.

[erlandsona]

Consider removing Relude_Unsafe from Relude.Globals to avoid shadowing in the event developers want to define their own Unsafe module for externals to js functions that can throw RTE's. We could rename ours to like Risky.re or something but convention for fn's that can throw is to prefix them with Unsafe so I'd prefer if we could define our own unsafe module but still open Relude.Globals at the top of files. Generally I'm fine with qualifying Relude.Unsafe.coerce for the few times we need that function. Alternatively expose coerce at the top-level of Relude.Globals like composition >>?

[andywhite37]

This is basically the problem with a global open Relude.Globals - it will never satisfy everyone and that's why I resisted it initially. I would recommend creating your own Globals that maybe re-aliases whatever you want and use that in your app. I don't think we're going to go down the road of trying to perfectly tune Relude.Globals to fit whatever arbitrary things people want.

I'll leave this open for more comments, but I'm inclined to close it.

[johnhaley81]

@erlandsona you could wrap Relude.Globals in a file and then use a destructive assignment to the module include to remove Unsafe.

// InternalReludeGlobals.re
include (Relude.Globals:  (module type of Relude.Globals with module Unsafe := Relude.Globals.Unsafe);

And then include that file instead of Relude.Globals. You can do that in bsconfig.json if you want: "bsc-flags": ["-open InternalReludeGlobals"]

[mlms13]

^^ That seems like an excellent guide for us to add to the docs.

In this specific case, I'm not 100% opposed to removing Unsafe from Globals, because Unsafe is hopefully not something people are reaching for often. However, it's technically a breaking change that we shouldn't take lightly, and I agree with @andywhite37 that I don't really want to be in the business of modifying Globals to work around app-specific contexts.

[andywhite37]

Yeah, @johnhaley81's suggestion seems great. I didn't know you could do that, but that seems like a good workaround.

[erlandsona]

In any case, I'm glad I asked because I'm probably gonna go down the route @johnhaley81's suggesting! @mlms13 glad you're still alive! Haha

[andywhite37]

Thanks for asking, I learned something too!

[erlandsona]

Feel free to close this if you feel so inclined 👌

[flash-gordon]

IMO it would be better to have "unsafe" prefix in function names. I don't think grouping functions by "unsafety" in a module makes a lot of sense. On the other hand, 1 function is not enough to make any strong judgment :)