Skip to content

Convert settings structs to consuming builders#321

Merged
kornelski merged 2 commits into
mainfrom
less-breaking-change
May 22, 2026
Merged

Convert settings structs to consuming builders#321
kornelski merged 2 commits into
mainfrom
less-breaking-change

Conversation

@orium
Copy link
Copy Markdown
Member

@orium orium commented May 22, 2026

Two changes that should land before v3.0.0 is released:

  1. b80b48e — Rename the integration_test Cargo feature to _integration_test.
  2. 6922879 — Convert Settings, MemorySettings, and RewriteStrSettings to a private-fields + consuming-builder API.

Why

We bumped v2 → v3 because we needed to add fields to Settings and MemorySettings, and (since those structs had all-pub fields and were not #[non_exhaustive]) any field addition is a major-breaking change for downstream callers using struct literals. We hit this twice on the way to v3.0.0; this PR closes the door so it doesn't happen again.

1. _integration_test rename

cargo-semver-checks's default heuristic enables every Cargo feature except ones it recognises as internal (unstable, nightly, bench, no_std, or names starting with _, unstable_, unstable-). The integration_test feature didn't match, so the heuristic enabled it during semver analysis, exposed the gated pub use re-exports as part of the public API, and produced spurious breakage reports for items like TransformStreamSettings that are not actually reachable from downstream crates.

Renaming to _integration_test tells those tools the feature is internal. The feature was already documented as "Unstable: for internal use only" and is only re-exported in lib.rs behind cfg(feature = …), so this has no effect on the actual public API. The constructive byproduct is the spurious reports go away.

2. Settings → consuming builders

Settings, MemorySettings, and RewriteStrSettings now have pub(crate) fields and expose a consuming-builder API:

  1. #[must_use] pub const fn with_<field>(self, value) -> Self setters for every scalar field (const where feasible).
  2. #[must_use] pub fn append_<handler>(self, …) -> Self for the two Vec fields. The argument type matches what the existing element! / comments! / text! / doctype! / doc_text! / doc_comments! / end! macros already produce, so the macro outputs can be passed directly.
  3. No with_* variant for Vec fields. Callers with a pre-built Vec (e.g. the C API, which receives handlers from FFI) use a loop, which is fine — Vec replacement is uncommon enough that we don't want the extra API surface.
  4. #[non_exhaustive] would be redundant once fields are private, so it's not used.

Migration shape:

// Before
Settings {
    element_content_handlers: vec![element!("div", |el| { /* … */ })],
    strict: false,
    ..Settings::new()
}
// After
Settings::new()
    .with_strict(false)
    .append_element_content_handler(element!("div", |el| { /* … */ }))

All in-tree callers migrated: lib unit tests + doctests, c-api, js-api, fuzz test case, integration tests, examples, benches, and the README. The c-api uses the documented loop pattern; the js-api swaps the in-progress Settings out via std::mem::take so it can keep applying the consuming builder while sitting behind a &mut.

orium added 2 commits May 22, 2026 16:31
The leading underscore signals "internal" to `cargo-semver-checks` (and similar
tools), which by default enables every feature that doesn't match its internal-name
heuristic (`unstable`, `nightly`, `bench`, `no_std`, or names starting with `_`,
`unstable_`, `unstable-`). Before the rename, the heuristic enabled the feature
and analyzed the gated `pub use` re-exports as part of the public API, producing
spurious breakage reports for items like `TransformStreamSettings` that are not
reachable from downstream crates.

The feature was already documented as "Unstable: for internal use only" and is
only re-exported in `lib.rs` behind `cfg(feature = ...)`, so renaming it has no
effect on the actual public API.
`Settings`, `MemorySettings`, and `RewriteStrSettings` previously exposed all their
fields as `pub`, which made adding a new field a SemVer-breaking change for any
downstream caller using struct-literal construction (and we hit that twice on the way
to v3.0.0). Switch them over to a private-fields + consuming-builder API so future
field additions can be minor changes.

1. Fields are now `pub(crate)`. External code can no longer use struct literals or
   field assignment to construct or configure these types.
2. Each scalar field gets a `#[must_use] pub const fn with_<field>(self, value) -> Self`
   setter (where `const` is feasible).
3. The two Vec fields (`element_content_handlers`, `document_content_handlers`) get
   `#[must_use] pub fn append_<handler>(self, …) -> Self` methods. The argument type
   matches what the existing `element!` / `comments!` / `text!` / `doctype!` /
   `doc_text!` / `doc_comments!` / `end!` macros already produce, so the macro outputs
   can be passed directly.
4. No `with_*` variant is offered for Vec fields. Callers with a pre-built `Vec` (e.g.
   the C API, which receives handlers from FFI) use a loop, which is fine — Vec
   replacement is uncommon enough that we don't want the API surface.
5. `#[non_exhaustive]` is redundant once fields are private and is not used.
6. All callers in-tree migrated: lib unit tests + doctests, c-api, js-api, fuzz test
   case, integration tests, examples, benches, and the README. The c-api uses the
   documented loop pattern; the js-api swaps the in-progress `Settings` out via
   `std::mem::take` so it can keep applying the consuming builder while sitting behind
   a `&mut`.

Verified with the full test suite (lib + doctests + integration), `cargo clippy --all
--all-targets` with and without `_integration_test`, `cargo doc`, the C API `prove`
suite, and `cargo semver-checks --release-type minor --only-explicit-features` against
v2.9.0 — which now flags exactly the 4 expected major-breaking changes
(`constructible_struct_adds_private_field`, `struct_pub_field_missing`,
`struct_pub_field_now_doc_hidden`, plus the previous `feature_missing` from the
`_integration_test` rename), all covered by the v3 bump.
@orium orium requested review from a team, Noah-Kennedy and jasnell as code owners May 22, 2026 17:11
@kornelski kornelski merged commit b00626a into main May 22, 2026
6 checks passed
@kornelski kornelski deleted the less-breaking-change branch May 22, 2026 18:21
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