Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 128 additions & 0 deletions API-SURFACES.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
# RFC: Reduce the per-module API surface count

Status: **proposed** — not started. Written as a hand-off for a follow-up session.
Prompted by review of #128 (the dual-API / `BeamChardata` work).

## The concern

A consumer of a single binding module today meets up to **three** distinct API surfaces. For
`Fable.Beam.String`:

| Surface | Style | Examples |
| --- | --- | --- |
| `str.*` (ImportAll interface) | tupled, member access | `str.lowercase "x"`, `str.trim (" x ", dir)`, `str.slice ("x", 0, 3)` |
| module helpers | curried, module function | `reverse "x"`, `pad "x" 5`, `find "x" "y"`, `splitAll "a,b" ","` |
| `*Raw` variants | curried, module function | `padRaw "x" 5`, `splitAllRaw "a,b" ","` |

Every stdlib module has this shape: `maps.get`/`maps.put` (interface) alongside `tryFind`/`ofList`/
`keysRaw` (module); likewise `lists`, `binary`, `re`, `proplists`.

## What is and isn't actually wrong

- **The `*Raw` layer is fine.** It is opt-in, self-documenting, and clearly derived from its friendly
sibling — a deliberate BEAM-native escape hatch (see the "Dual API" section of
`BINDINGS-GUIDE.md`). Nobody reaches for `padRaw` by accident. This RFC does **not** propose
changing it.
- **The wart is the `xxx.*`-interface vs. module-helper split**, and it predates the dual-API work.
The boundary is drawn by an *implementation detail*, not by anything a caller can see: if the raw
Erlang return maps straight to an F# type it becomes an ImportAll member (`str.lowercase` — binary
→ string, no wrapper); if it needs an Emit wrapper — a `Result`, a `characters_to_binary` flatten,
a `new_ref` array wrap — it becomes a module `[<Emit>]` function (`reverse`, `find`, `splitAll`).
So `str.lowercase` and `reverse` are the same *kind* of thing to a caller, yet live on different
surfaces with different call conventions. That arbitrariness is the problem.

So this is really a **two-surface** problem (arbitrary interface/module split) plus one justified
escape hatch — not three equally-arbitrary surfaces.

## Why it is this way (history)

`[<ImportAll>]` is a cheap way to bind many functions of a module at once, but ImportAll codegen
emits a bare `module:function(args)` call — it cannot insert a wrapper. So any binding needing a
non-trivial return (Result, option, flatten, ref-wrap) *had* to be a module-level `[<Emit>]`
function. The interface caught the rest. Nothing chose the split deliberately; it fell out of the two
mechanisms.

Note: `[<Emit>]` **can** decorate an interface member (it overrides ImportAll codegen for that one
method — see `maps.keys`, `BINDINGS-GUIDE.md` "Erlang lists vs F# arrays"). So the split is not
forced by the tooling — either surface *can* host any binding. Which means we can collapse it.

## Options

**A. Unify onto curried module functions (recommended).** Drop the `xxx` ImportAll interfaces (or
demote them to an internal/raw escape hatch), and expose every friendly operation as a curried
module-level `[<Emit>]` function. Result: one friendly surface + `*Raw` where a BEAM-native form
exists.
- Pro: curried is F#-idiomatic and pipe-friendly (`s |> trim |> reverse`); matches where the typed
helpers already live; one mental model.
- Con: overloaded arities lose overloading and become distinct names (`slice`/`sliceLen`,
`trim`/`trimDir`, `equal`/`equalCaseInsensitive`) — the same trade `pad`/`padDir`/`padWith`
already made. More `[<Emit>]` lines than interface members.

**B. Unify onto the ImportAll interface (tupled).** Put every binding on `str.*` etc., using
`[<Emit>]` on members for the wrapped ones.
- Pro: keeps arity overloading; one binding block per module.
- Con: tupled args don't pipe; less idiomatic F#; a `BeamChardata`/`Result`-returning member sits
visually next to `string`-returning ones with no cue about the different return — the exact
"looks like it returns a string" trap that motivated the chardata fixes.

**C. Do nothing; document the split.** Add a short "why two surfaces" note to `BINDINGS-GUIDE.md` so
the pattern is at least predictable. Cheapest; leaves the arbitrariness in place.

## Recommendation

**Option A**, done deliberately and repo-wide, but **phased and prototyped first**. It gives the
cleanest end state — "friendly curried functions + `*Raw` escape hatch" — and aligns the whole
library on one call convention.

Suggested sequence:
1. Prototype on `String.fs` only. Produce a before/after and measure the call-site churn (tests,
`spike/`, Cowboy, synapse). Decide go/no-go from real numbers, not this doc.
2. If go: convert module by module, each its own commit. Keep `*Raw` variants unchanged.
3. Update `BINDINGS-GUIDE.md` (the "Quick Reference", the `[<ImportAll>]`-vs-`[<Emit>]` guidance, and
the module-file template) to make curried module functions the house style, and mark ImportAll as
the raw/escape-hatch mechanism only.

## Impact / cautions

- **Breaking, repo-wide.** Every `str.pad`/`maps.get` call site flips tupled → curried. Pre-1.0
(`-rc.x`), so breaking is acceptable, but it touches this repo, the `Fable.Beam.Cowboy` package, and
the downstream `synapse` app. Grep synapse's call sites before committing to it.
- **Naming churn** from lost overloads (see Option A cons). Agree the naming scheme up front
(`sliceLen`, `trimDir`, `equalCaseInsensitive`, …) so it's consistent across modules.
- **Not a rider on any feature PR.** This is its own initiative with its own review.

## Concrete sketch — `String.fs`, before → after (Option A)

```fsharp
// before
str.lowercase "HELLO" // interface, tupled
str.slice ("hello world", 0, 5) // interface, overloaded arity
str.trim (" x ", leading) // interface, overloaded arity
reverse "hello" // module, curried
pad "hi" 5 // module, curried

// after — one curried surface
lowercase "HELLO"
slice "hello world" 0 // slice/2
sliceLen "hello world" 0 5 // slice/3 -> distinct name
trim " x " // trim/1
trimDir " x " leading // trim/2 -> distinct name
reverse "hello" // unchanged
pad "hi" 5 // unchanged
// raw escape hatch unchanged: reverseRaw, padRaw, splitAllRaw, ...
```

## Open questions for the follow-up session

1. Do we keep a raw ImportAll interface per module (e.g. `str`) as a documented escape hatch for
arbitrary OTP calls, or drop it entirely?
2. Naming scheme for de-overloaded arities — settle it once, apply everywhere.
3. Phasing: all modules in one PR, or a stack of per-module PRs (easier review, longer breakage
window on `main`)?
4. Is the churn to `synapse` acceptable, and should we land a synapse-side migration in the same
change or right after?

## Starting point

Begin at `src/otp/String.fs` (smallest self-contained case with all three surfaces) and its test
`test/TestString.fs`. The `pad`/`padDir`/`padWith` split already models the de-overloading approach.
86 changes: 86 additions & 0 deletions BINDINGS-GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ When writing or reviewing a binding, check:
| `[<Erase>] + [<ImportAll>]` | Binding an Erlang module with multiple functions | `timer`, `gen_server` |
| `[<Erase>]` on DU | Opaque Erlang types (compile-time safety, no runtime cost) | `Pid`, `Ref`, `TableId` |
| `[<Erase>]` on generic DU | Typed Erlang containers (maps, lists) | `BeamMap<'K,'V>`, `BeamList<'T>` |
| Flattened default + `*Raw` variant | Functions returning chardata/iodata or raw lists | `string:pad` (→ `string`) + `padRaw` (→ `BeamChardata`) — see "Dual API" |
| `[<Emit>]` on abstract member | Override `ImportAll` codegen for specific methods | `fable_utils:new_ref(...)` wrapping |
| `System.Func<>` / `System.Action` | Typed callbacks in `ImportAll` interfaces | `fold`, `filter`, `foreach` |
| `U2<>` / `U3<>` / erased union | Parameters or returns that accept multiple types | timeout: `int` or `infinity` |
Expand Down Expand Up @@ -60,6 +61,7 @@ The only narrow places `obj` is acceptable are listed in the "When `obj` is acce
| ---------------- | --------------------------- | ------------------------------------------ |
| `int`, `float` | `integer()`, `float()` | Direct |
| `string` | `binary()` | `<<"hello">>` — **not** charlists |
| `BeamChardata` | `unicode:chardata()` | binary/charlist/iolist; `*Raw` return, flatten with `unicode:characters_to_binary` — see "Dual API" |
| `bool` | `true` \| `false` | Atoms |
| `unit` | `ok` | Atom — but `Ok ()` in a `Result` is `{ok, ok}` (see bare-`ok` anti-pattern) |
| `tuple` | `tuple` | `{A, B, C}` — direct mapping |
Expand Down Expand Up @@ -433,6 +435,7 @@ For everything else that feels "polymorphic":
Is the Erlang type...
├── a number? → int, float, int64
├── a binary/string? → string
├── chardata (iolist/charlist)? → string (default, flatten) + BeamChardata for a *Raw variant
├── a boolean atom? → bool
├── the atom 'ok'? → unit
├── a fixed-size tuple? → T1 * T2 * ... (or Decode.tuple2/3 from Dynamic)
Expand Down Expand Up @@ -742,6 +745,68 @@ end)()
let getCwd () : Result<string, string> = nativeOnly
```

## Dual API: F#-friendly default + BEAM-native `*Raw`

Some OTP functions return a value in a *BEAM-native* form that is efficient and composable when you
are authoring Erlang, but awkward in idiomatic F#. Two cases recur:

- **chardata** (`unicode:chardata()`) — a binary, a charlist, or a nested iolist. Returned by
`string:pad`/`replace`/`reverse`, `uri_string:compose_query`, `io_lib:format`, and friends. It is
valid anywhere a binary or iodata is accepted (`io:format`, `gen_tcp:send`, Cowboy response
bodies), so it can be passed straight on without flattening. Keeping it unflattened and flattening
once at the I/O boundary is the idiomatic way to build output without repeatedly copying binaries.
- **raw Erlang lists** — a plain linked list, as returned by `maps:keys`, `string:split`,
`re:split`. F# array operations need it ref-wrapped first (see "Erlang lists vs F# arrays").

For these, bind the function **twice**:

- the **default** (plain name) returns the F#-friendly form — a flattened `string`, or a ref-wrapped
`'T array` — because that is what an F# consumer expects to compare, store, and pattern-match;
- a **`<name>Raw`** variant returns the BEAM-native type — `BeamChardata` or `BeamList<'T>` — for
zero-copy BEAM output and interop with hand-written Erlang.

```fsharp
/// Pads String on the trailing side to at least Length grapheme clusters.
[<Emit("unicode:characters_to_binary(string:pad($0, $1))")>]
let pad (s: string) (length: int) : string = nativeOnly

/// Like `pad`, but returns the raw chardata without flattening. See `BeamChardata`.
[<Emit("string:pad($0, $1)")>]
let padRaw (s: string) (length: int) : BeamChardata = nativeOnly
```

`BeamChardata` (in `Types.fs`) is an erased `unicode:chardata()` with two conversions:

```fsharp
[<Erase>]
type BeamChardata = BeamChardata of obj

BeamChardata.ofString: string -> BeamChardata // a binary is already valid chardata (zero cost)
BeamChardata.toString: BeamChardata -> string // flatten via unicode:characters_to_binary
```

Guidelines:

- **The default is the flattened / F#-friendly one.** Reach for `*Raw` only when you specifically
want the native form — most callers want the default.
- **Don't invent a `*Raw` where the function already returns the F# type.** `string:lowercase` /
`uppercase` / `trim` / `slice` return a binary for binary input, so there is no raw chardata form
to expose. Verify in `erl` before adding one.
- **The raw form is the *honest* return; the default's conversion is what makes its `string` /
`array` signature true.** A default that claims `string` but skips the flatten is silently wrong:
`string:pad("hi", 5)` is `[<<"hi">>,32,32,32]`, which compares unequal to `<<"hi ">>` yet prints
the same — the kind of bug that only surfaces once the value is compared, matched, or stored.
- **Pick the honest native type.** Only `reverse` yields a real charlist; `pad` / `replace` yield
iodata (nested binaries and integers). `BeamChardata` covers all of them — do not use
`BeamList<char>`, which would misrepresent iodata as a list of chars.

Where it applies:

| Native form | Default (F#) | `*Raw` (BEAM) | Applied |
| --- | --- | --- | --- |
| chardata | `string` | `BeamChardata` | `string:reverse`/`pad`/`replace`, `uri_string:compose_query`, `io_lib:format` |
| raw list | `'T array` | `BeamList<'T>` | `maps:keys`/`values`/`to_list`, `string:split`, `binary:split`, `re:split` (all 4 arities), `proplists:get_keys` |

## IIFE Wrapping for Variable Scoping

**As of Fable 5.0.0 this is handled automatically** — the Erlang backend wraps every
Expand Down Expand Up @@ -1040,6 +1105,27 @@ that shape matches Fable's `Result` representation directly, so no `[<Emit>]` is
**Always add a test that exercises the success (`ok`) path** of any bare-`ok` binding —
that is the only path that reveals a missing wrapper.

### Anti-pattern: asserting an implementation-defined value instead of the invariant

Some OTP functions return a value that is correct but **not portable** — it varies with the OTP
release, how the term was constructed, or VM internals. A test that pins the exact value passes on the
box it was written on and fails elsewhere. `binary:referenced_byte_size/1` is the canonical example:
it reports the size of the *underlying* memory a (sub-)binary references, which the docs call "a hint
for optimization, not exact". For `"hello"` it returned `5` on OTP 25, `40` on OTP 27, and `256` for
a shell literal.

```fsharp
// BAD — passes on OTP 25, fails on OTP 27. The binding is fine; the assertion isn't portable.
binary.referenced_byte_size "hello" |> equal 5

// GOOD — assert the guarantee the function actually makes: it references at least what it contains.
(binary.referenced_byte_size "hello" >= Erlang.byteSize "hello") |> equal true
```

When a function's exact result is implementation-defined, assert the **invariant** it guarantees
(a bound, an ordering-independent property, a round-trip), not a value observed on one runtime.
Verify in `erl` across OTP versions if you are unsure which part of the result is contractual.

## Module File Template

```fsharp
Expand Down
Loading
Loading