Update to Bastet

[mlms13]

• Update bs-platform to 7.2.2
• Update bs-abstract to bs-bastet@1.2.3
• Update tons of BsAbstract references to BsBastet (I tried to factor out repeated BsAbstract.Interface. into opens in cases where it didn't conflict with anything)
• There are a couple Extensions.* modules that re-aliased ordering, and now they just use the definition from BsBastet.Interface instead (which is technically a breaking change)

This PR doesn't (yet) build on native, but at this point it should just be a matter of splitting the JS-specific stuff into its own separate library. Also, it might be worth waiting for Risto-Stevcev/bastet#26 to be fixed, as currently the branch doesn't build unless you manually install bisect_ppx.

[mlms13]

Ah yeah, I was messing around with tests, mostly to see how it affected the output JS, separate from the Bastet change. The O(1) Labs folks seem to use the "structural typeclass" approach in their modular-explicits PR, and I kind of like it, but I can revert the tests if you prefer.

[andywhite37]

I'd personally prefer to keep those explicit instance modules, even in the tests, mainly because that's how the library is setup, and I'd like for there to be consistency. I also prefer to have the explicit declaration that a set of types and functions was intended to implement a typeclass, for both the compiler to check and for readers/users of the code. I think there's probably something to the idea of having inferred typeclass instances, but I think it also brings about some issues and inconsistencies, and don't want to move in that direction for the library as it is structured now. Maybe that's a "relude v2" type of thing, or a greenfield standard library, that takes a different approach. I think there is historical precedent for having explicit modules too, so I don't think having them is non-idiomatic Ocaml.

We can discuss more, but I'd like to keep things consistent. E.g. if we want to rename something, add interfaces, restructure things, etc. I'd like to do it consistently. If any of these things means a big refactor/restructure of the library, I'd like to make a concerted effort to make the change across the board. I will promise that if I'm ever tempted to do a big restructure, I will open a discussion about it first. (Not saying that you did anything wrong, just that I will commit to discussion/collaboration for any big changes in the future, as I'm requesting here.)

[mlms13]

This reverts to explicit modules/signatures in test, and it further bumps bs-bastet to the latest release, which includes a fix for #252 and has the proper dependencies on bisect_ppx. @andywhite37 I'm curious to see if that ppx gives you trouble on NixOS.

[andywhite37]

Cool - thanks for bringing those instance things back. I didn't want that to hold you back, but thanks for making the change anyway.

I'll try it out on nixos - I guess the question is where that comes into play with the npm install - like if bastet needs to run that tool as part of the install. I'll try it out at some point. If there's a problem I'll just try to figure out a workaround or see if anyone has a good fix.

[mlms13]

My understanding is that Bastet only really needs the PPX as a dev tool (and shouldn't actually run it in our case), but that Bucklescript doesn't yet have the concept of a dev ppx, so it still needs to be included as a real dependency. This means the ppx will end up in downstream node_modules, but it might not actually ever do anything?

If it doesn't give you too much trouble, I feel pretty ready to merge and release this.

[andywhite37]

Yeah, it only matters if something tries to run it, like a postinstall script. If it just comes with bs-bastet as a broken binary for me, it doesn't matter. It would be nice to have a general approach for these pre-compiled binaries, but I've only been caring about things that affect important workflows.

[andywhite37]

Feel free to merge whenever you're ready - I can figure out any nix issues later.

[mlms13]

I just pointed some of our relude-adjascent projects (relude-parse, bs-decode, relude-eon) at this PR and ran their tests, and there were no hiccups, so I'm going to merge and release this thing (followed by the others).