feat: remove Belt dependency from Stdlib
#796
Conversation
| @@ -96,26 +96,14 @@ let public_method_label s : tag = | |||
| (**** Sparse array ****) | |||
|
|
|||
| module Vars = | |||
| #ifdef BS | |||
| Belt.Map.String | |||
There was a problem hiding this comment.
Users of the standard library will lose this "optimization" to not use a functor, do we want to inline Belt.Map.String perhaps?
There was a problem hiding this comment.
This is a very good question, that I hope I can answer sufficiently:
- I think short term the answer is "we don't care" because the benefits (removing belt as a dep) justify it, and the code becomes the same as the upstream stdlib
- even if it did, I don't think the impact would be important -- and Melange users barely use OCaml objects anyway.
- longer term we want to add continuous benchmarks to melange, which we've added to the next quarter roadmap. Hopefully we can measure the impact of this change retroactively then.
There was a problem hiding this comment.
and Melange users barely use OCaml objects anyway.
Not sure I get this part, could you elaborate? How are objects related to the discussion about larger bundles when using functors?
There was a problem hiding this comment.
Oh, I see, this is the camlinternalOO module 🤦
anmonteiro
force-pushed
the
anmonteiro/untangle-belt-from-stdlib
branch
from
8c96650 to
bd8ff78
Compare
| Unreleased | ||
| --------------- | ||
|
|
||
| - Remove `Belt` as a dependency of `Stdlib` |
There was a problem hiding this comment.
This is a breaking change. Most projects use Dune default config for implicit transitive deps, so they will have to add melange.belt to the list of libraries after upgrading. See melange-re/melange-re.github.io#140.
There was a problem hiding this comment.
will mark it as such. thanks!
melange.beltas a dependency for the melange stdlibmelange.beltto the libraries that melange auto-includes by default