Skip to content

fix: Loader identify reform - #6925

Open
ef4 wants to merge 17 commits into
mainfrom
loader-identify-reform
Open

ef4 wants to merge 17 commits into
mainfrom
loader-identify-reform

Conversation

@ef4

@ef4 ef4 commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

This switches our strategy for identifying which module a card/field definition comes from.

Before it was a custom module loader extension, now it is a build step that tags classes with a symbol.

Since we already have required custom build steps, this is an overall reduction in ways we diverge from standard module loading, and it eliminates special handling that was previously required for base realm files inside host.

@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Preview deployments

Host Test Results

480 tests   478 ✅  8m 24s ⏱️
  1 suites    2 💤
  1 files      0 ❌

Results for commit d9f5424.

Realm Server Test Results

    1 files    364 suites   2h 14m 12s ⏱️
6 064 tests 6 064 ✅ 0 💤 0 ❌
6 127 runs  6 127 ✅ 0 💤 0 ❌

Results for commit d9f5424.

@ef4 ef4 changed the title Loader identify reform fix: Loader identify reform Oct 22, 2026
ef4 and others added 13 commits October 23, 2026 11:51
boxel-cli's local-mount server now passes each module's URL to transpileJS,
so the classes it serves carry identity tags like a realm-server's do.

Host tests tag their fixture classes as a loader records a module's exports,
covering every shim path rather than only the test adapter's, and tagging a
class with the first module its loader credits it to.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A class a factory makes, as in `export const Status = enumField(...)`, has
no body to put the identity member in. The plugin now follows such a binding
with a statement that tags its value once it exists, when that value is a
function carrying no identity of its own. `export default makeClass()` is
bound to a name first so the statement can reach it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The gallery job paired this commit's host harness with a published CLI's
test engine. The engine compiles the realm's modules for the host, which now
identifies the classes they export by what the compiler records in them, so
a published CLI's compiler no longer fits a newer host. Build the CLI here
instead of installing a release.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Compiled modules are cached in-process, in module_transpile_cache, and by
clients holding their validators, all keyed by source alone. Tagging exported
classes changes what unchanged source compiles to, so output from before it
would outlive the change until a reindex wiped the cache. The module ETag now
carries a transpiler version: a row or validator from an earlier version no
longer matches and the module is compiled over.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A realm now tags its modules with their URL, and the loader canonicalizes the
tag when it reads it. What a module compiles to no longer depends on the
realm prefixes registered at compile time, so a prefix registered later
spells the module the same way identify() does.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
setupTestRealm tags the classes in its module contents under the module's
realm URL, and tests that shim modules directly go through shimTestModule (or
tagTestModuleExports for a lazily resolved shim). A class keeps the first tag
it gets in a test, which is the module a loader credits it to.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Both realms in the operator-mode acceptance setup exported one class object
as `person`, which no two modules can do: a class is declared by one of them.
The second realm now exports a subclass, so each class's identity names the
module that declares it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
identify() now answers only from the identity tag compiled into a class,
canonicalized through the loader's virtual network. The loader no longer
records an identity for every function a module exports, and the bundled-base
identity registry is gone, along with the build step that published it and the
assertion that compared the two systems. getLoaderFor keeps the record of which
loader loaded each export.

The bundled-base tests now assert what compile-time tags make true: a class
only a bundled sibling names is identified by the module that declares it,
whichever modules the loader was asked for.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
identify() built a new object from a class's tag on every call, where the
loader's record it replaced handed back one object per class. Callers compared
identities by reference: the new-card menu's de-duplication of recent card
types listed a type twice and dropped the next one. identify() now returns one
object per class for as long as its module canonicalizes the same way, and the
menu compares types by module and name.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@ef4
ef4 force-pushed the loader-identify-reform branch from 318ccaf to a2d473c Compare October 23, 2026 15:51
ef4 and others added 2 commits October 23, 2026 12:17
It was written for the bundled-base identity registry, which is gone. Its
fixtures now tag their own classes, so what it exercises is the test helper;
a base class identified wrongly fails the many tests that load base cards.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The plugin tagged the value of any exported binding computed by a call or a
reference, which emitted a runtime statement for template-only components,
modifiers, symbols and data alongside the handful of factory-made classes.
It now tags an exported binding only when its value is a call to a listed
class factory, named by module and export: base's enumField and the catalog's
statusField and priorityField. A call is recognized however the module
reaches the factory: by a default, named or namespace import, by a realm
alias or relative specifier, or as a function the module exports itself.

The host names base modules, and resolves base's relative imports, with
@embroider/reverse-exports; the realm canonicalizes imports through its
virtual network.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Comment thread .github/workflows/ci.yaml
if: needs.change-check.outputs.choreo == 'true' || github.ref == 'refs/heads/main' || needs.change-check.outputs.run_all == 'true'
runs-on: ubuntu-latest
timeout-minutes: 15
timeout-minutes: 25

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR changes some tests to build boxes-cli instead of consuming the published version. That probably slows down CI and is undesirable, but it's necessary until a published boxer-cli is compatible with the changes in this PR.

let ref = identifyCard(card.constructor);
let name = cardTypeDisplayName(card);
if (isResolvedCodeRef(ref)) {
if (items.find((item) => item.ref === ref && item.name === name)) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implicitly assumed that we could trust object stability of ref. Mostly we do have that stability, because we're efficient about reusing objects, but it's more correct to compare by value.

Comment thread .github/workflows/ci.yaml
if: needs.change-check.outputs.choreo == 'true' || github.ref == 'refs/heads/main' || needs.change-check.outputs.run_all == 'true'
runs-on: ubuntu-latest
timeout-minutes: 15
timeout-minutes: 25

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR switches to building boxel-cli in CI instead of consuming the published one, because the published one is not compatible with our changes.

We could go back to using the published one after this lands and we publish.

// The second realm's module declares a class of its own, as any two
// modules do: one class object can only have been declared by one of
// them.
class SecondRealmPerson extends Person {}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests were creating a situation that's impossible in real realms -- the same class can't actually be declared in two different realms.

ef4 and others added 2 commits October 23, 2026 12:43
The plugin no longer carries a list of class factories of its own; it tags
the results of the ones its caller names. The realm's transpile and the
host's babel config both pass the list in runtime-common/class-factories.ts.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Resolving the base realm's alias inside the plugin made it import the realm
prefixes, which brought the base card types into every package that
type-checks the transpiler, and failed boxel-cli's lint. The plugin now only
resolves relative imports; callers spell other forms through canonicalModule.
The realm canonicalizes through its virtual network, and a compile without
one maps the base realm's alias, beside the factory list.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@ef4

ef4 commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

This will see CI failures until #6936 is landed.

@ef4
ef4 marked this pull request as ready for review October 23, 2026 17:16
@ef4

ef4 commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

@lukemelia this is ready for review and merge, it's the followup from my latest work with @backspace

This branch has not been deployed

No deployments
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.

1 participant