Skip to content

Audit the public export surface and deprecate internal helpers ahead of 7.0 #551

Description

@cjbarth

Summary

The package publishes 40 symbols, and a good share of them are internal helpers that became public by accident rather than by design. Now that src/index.ts lists its exports explicitly instead of using export * from, we can see the whole surface and decide what belongs in it.

The goal for 7.0 is to shrink the surface. This issue covers the audit and the 6.x deprecation pass that has to come first — a name cannot simply vanish in 7.0 without consumers having seen a warning.

How the surface got this wide

src/index.ts did:

export * from "./types";
export * from "./utils";

Every module-level export in utils.ts was therefore public automatically. Adding a helper there — even one intended purely for use by signed-xml.ts — silently widened the package's API, because a sibling module and an external consumer reach it through the same export keyword. That is how findChilds stayed published with no callers, and how each new helper joined the surface without anyone choosing to publish it.

Current surface

40 symbols: 18 types and 1 value from types.ts, 16 values from utils.ts, plus SignedXml and the four canonicalization classes.

The 18 types are genuinely consumer-facing — dependents such as node-saml import Reference, SignedXmlOptions and friends directly. Expect that block to survive mostly intact. The utils block is where the work is:

Export Callers outside utils.ts Notes
findChilds 0 deprecated alias; see #550
isArrayHasLength 17 internal predicate
findChildren 16 internal DOM helper
findAttr 5 internal DOM helper
encodeSpecialCharactersInText 5 internal serialization helper
isDescendantOf 4 internal DOM helper
encodeSpecialCharactersInAttribute 3 internal serialization helper
validateDigestValue 3 internal; constant-time comparison
findAncestorNs 3 plausibly useful to implementors
EXTRACT_X509_CERTS 2 internal regex
derToPem 2 plausibly useful to consumers
pemToDer 2 plausibly useful to consumers
findAncestorNsForNode 2 added recently; internal
BASE64_REGEX 1 internal regex
PEM_FORMAT_REGEX 1 internal regex
normalizePem 1 plausibly useful to consumers

Rough grouping to argue about, not a decision:

  • Almost certainly internal: the three regexes, isArrayHasLength, findAttr, findChildren, isDescendantOf, both encodeSpecialCharacters*, findAncestorNsForNode.
  • Worth keeping or reshaping: derToPem, pemToDer, normalizePem — key-format conversion is a real consumer need. findAncestorNs is useful to anyone implementing a custom canonicalization algorithm.
  • Needs care: validateDigestValue performs the constant-time digest comparison. If anything external depends on it, removing it may push someone toward a naive ===. Check before dropping.

A caveat on the call counts: they measure our usage, not consumers'. Low internal use does not prove nobody imports it. Worth a look at how dependents actually import from this package before committing to a list — node-saml first, since a break there is a break for most of the ecosystem.

Proposed approach

  1. Decide which names stay. The table above is a starting point, not a conclusion.

  2. In a 6.x release, mark everything on the removal list @deprecated with a replacement named, plus a runtime warning so JavaScript consumers see it too. House pattern, from src/signed-xml.ts:

    import { deprecate } from "util";
    
    const warnX = deprecate(() => {}, "`x()` is deprecated and will be removed in a future version. Use `y()` instead.", "XML_CRYPTO_X");

    A bare /** @deprecated */ with no replacement and no runtime warning — what findChilds has today — is not enough. It is invisible to JavaScript consumers, and it tells TypeScript users nothing about what to switch to.

  3. In 7.0, delete the deprecated entries from the export list in src/index.ts and drop the export keyword from the ones that stay internal to utils.ts.

  4. Document the removals and their replacements in the changelog and the README API section.

Guard against regrowth

Removing names once does not stop the surface growing back the same way. Worth considering:

  • An api-extractor report, or a snapshot test over the exported names, so that widening the surface shows up as a reviewable diff rather than a side effect of adding a helper.
  • Splitting genuinely-internal helpers into a module that index.ts never re-exports, so "public" becomes a deliberate act.

Related

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions