MemberJunction / MemberJunction/MJ

Guard MJ's public API surface and the Open App installer contract in CI

Open
#4,360 0 comments 0 reactions 1 assignee Claimed by @cadam11 View on GitHub
Dominant language
TSQL
Stars
29
Forks
6
Avg merge
2d 1h
Merged PRs (30d)
323

Description

## Why

During the 6.1.0 edge series two Open Apps broke on MJ upgrades, and neither break was visible to anyone in MJ's own CI.

**BCSaaS.** PR #3734 (edge.2) moved `UserCache` from `@memberjunction/sqlserver-dataprovider` to `@memberjunction/generic-database-provider` and deleted the old export on purpose, because `.claude/rules/typescript-style.md` prohibits cross-package re-exports. The changeset body says BREAKING in bold caps, and every bump in it was `patch`. BCSaaS failed to load on edge.2 and edge.3 until a `@deprecated` re-export was restored in edge.4 (commit 6cbed1d619). The same PR changed `Refresh` from taking a pool to taking a provider, and since `Refresh` swallows its own errors, BCSaaS's user cache refresh silently does nothing on every edge build through edge.5 (BlueCypress/SaaS#115).

**bizapps-common.** The Open App Engine's `dynamicPackages` idempotency check matched the whole config file rather than the target array, so a `shared` package written to `server` first never reached `client`, and its `@RegisterClass` components were tree-shaken out of the browser bundle with no error anywhere. Fixed in edge.5 (open-app-engine changeset 23c2521). bizapps-common has two shared packages and was exposed for the whole edge.0 through edge.4 window.

Breaking changes to MJ core mean a major, and devs cannot trigger a major, so a PR like #3734 has exactly one legal shape: keep a deprecated re-export until the next era. Nothing in CI notices a removed public export today, and the lint rule actively pushes authors toward deleting it. Both checks below live entirely in MJ. MJ knows about Open Apps as a concept; it must not have to know about any particular app.

## 1. Export-surface diff on every PR

For each package a PR touches, compare the top-level exports in the built `.d.ts` against the last published version of that package. A removed or re-signatured public export fails the build. There is no changeset or label escape hatch: the only way through is a `@deprecated` shim at the old location that lives until the next major. This protects every consumer, including the ones we have never heard of, which is the only compatibility mechanism a library publisher can actually offer.

## 2. Fixture Open App in the MJ repo, installed and booted in CI

A minimal Open App checked into this repo whose only purpose is to exercise the installer contract: at least one `shared` package that registers a client class with `@RegisterClass`, run through `mj app install` and a server plus client boot in CI, asserting the registration is reachable on both sides. This is the shape of bug that exposed bizapps-common, and a fixture tests the contract itself rather than any real app.

## 3. Carve-out in the re-export rule for deprecated compat shims

Amend the NO RE-EXPORTS BETWEEN PACKAGES section of `.claude/rules/typescript-style.md` with one exception: when a public export moves packages, the old location keeps a `@deprecated` re-export pointing at the new one, with a comment naming the era in which it is removed. The rule as written is what deleted the re-export in #3734, and the edge.4 fix had to violate it to restore loading.

## Out of scope

Testing MJ against specific downstream repos (bizapps-common, BCSaaS, or any other). See the discussion in MJ dev core, 2026-09-10.

## Implementation notes from the investigation

**Where the policy already lives.** `plans/lts-process.md` 3.3: "Majors keep their traditional meaning." Majors are era-scale and only the release engineer opens an era, so within an era the public surface of every `@memberjunction/*` package is additive-only. The gate in item 1 is the enforcement of a rule that is already written down.

**Resolving "last published version" for the diff.** The comparison target is whatever `npm view version` returns for the package's current dist-tag on the branch (`edge` on next, `lts-X.Y` on a line branch), fetched with `npm pack` and unpacked, never the workspace symlink. bizapps-common's PR #135 (`scripts/check-against-published-mj.mjs`) already does the resolve-pack-unpack half of this from the consumer side and is a reasonable reference. One known wrinkle from that work: `@memberjunction/server` publishes TypeScript source as its `types` entry (`"types": "./src/index.ts"`), so a `.d.ts` diff of that package needs to compile the source or compare the source's export statements directly.

**What counts as the surface.** Top-level `export` declarations of the package entry point: names, and for functions and methods the declared signature text. Type-only exports count. The comparison is new set minus old set; additions are free.

**What the type diff cannot catch, and why item 2 exists.** The `Refresh(pool)` call in BCSaaS compiled fine against the new signature because the argument was typed loosely on the consumer side, and the failure was a swallowed runtime error. The bizapps-common tree-shaking bug had no type signal at all. Both are runtime contract failures, which is what a booted fixture app catches and a declaration diff never will. The two items are complementary, not alternatives.

**Related diagnostic already shipped.** edge.5 also added a `ClassFactory.Register` collision warning for unrelated classes registered under the same `(base, key)` pair (see the same 23c2521 changeset). The fixture app in item 2 should assert that warning stays silent on a clean install, so it doubles as a regression test for the registry.

**Considered and rejected.** Installing bizapps-common and BCSaaS against every MJ publish as a compatibility check. It would have caught both incidents a week earlier, but it only protects the consumers we happen to own, ties MJ CI to repos it does not control, and is not how any library publisher can realistically work. The right shape is the two MJ-internal checks above; downstream teams keep their own pre-push check against published MJ (bizapps-common #135) for the reverse direction, code written against unpublished MJ.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.