Add more setup for RealmResourceIdentifier migration#4425
Add more setup for RealmResourceIdentifier migration#4425
Conversation
backspace
force-pushed
the
@cardstack/base-no-registerCardReferencePrefix-cs-10745
branch
from
592630d to
7df6350
Compare
VirtualNetwork.fetch now accepts scoped RRI strings (e.g.
"@cardstack/base/card-api") in addition to URLs and Requests. RRIs
are resolved to real URLs via realm mappings before creating the
Request, since new Request('@cardstack/base/...') would throw.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Returns the list of registered RealmIdentifiers from realm mappings. Non-network code can use this to check realm membership without needing to resolve RRIs to URLs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace registerCardReferencePrefix + addImportMap pairs with single addRealmMapping calls for catalog, skills, and openrouter realms. Add addRealmMapping for @cardstack/base/ scoped prefix (new). Keep addURLMapping for the fake https://cardstack.com/base/ URL mapping which is still needed during the transition. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace registerCardReferencePrefix + addImportMap pairs with single addRealmMapping calls in main.ts and worker.ts for non-URL prefix mappings (e.g. @cardstack/catalog/). URL-based mappings still use addURLMapping. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
backspace
changed the title
Preview deployments |
…chain Verifies that addRealmMapping + VirtualNetwork.fetch resolves @cardstack/base/card-api to http://localhost:4201/base/card-api through the complete middleware chain. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds additional infrastructure for migrating from URL-based realm addressing toward RealmIdentifier / RealmResourceIdentifier (RRI) support, centered around VirtualNetwork being able to register scoped realm prefixes and use them during fetch.
Changes:
- Added
VirtualNetwork.addRealmMapping()andVirtualNetwork.knownRealms()to register and introspect scoped realm prefix mappings (bridging to existing import-map and card-reference prefix registration). - Added support for
VirtualNetwork.fetch()to accept scoped RRI strings (e.g.@cardstack/base/card-api) by resolving them to real URLs before constructing aRequest. - Updated host + realm-server startup wiring to use
addRealmMapping()instead of manually callingregisterCardReferencePrefix()andaddImportMap(), and added tests for the new behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/runtime-common/virtual-network.ts | Introduces realm-prefix mapping registry, exposes known realms, and resolves RRI strings in fetch() |
| packages/realm-server/main.ts | Switches non-URL realm prefix wiring to virtualNetwork.addRealmMapping() |
| packages/realm-server/worker.ts | Switches non-URL realm prefix wiring to virtualNetwork.addRealmMapping() |
| packages/host/app/services/network.ts | Registers @cardstack/* scoped realm prefixes via addRealmMapping() while keeping base URL mapping |
| packages/realm-server/tests/card-reference-resolver-test.ts | Adds coverage for addRealmMapping(), knownRealms(), and RRI support in VirtualNetwork.fetch() |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| addRealmMapping(realmIdentifier: string, targetURL: string): void { | ||
| let normalizedId = ensureTrailingSlash(realmIdentifier); | ||
| let normalizedTarget = ensureTrailingSlash(targetURL); | ||
| this.realmMappings.set(normalizedId, normalizedTarget); | ||
|
|
||
| // Backward compat bridge: populate both existing registration systems | ||
| // so that resolveImport and resolveCardReference continue to work | ||
| this.addImportMap( | ||
| normalizedId, | ||
| (rest) => new URL(rest, normalizedTarget).href, | ||
| ); | ||
| registerCardReferencePrefix(normalizedId, normalizedTarget); | ||
| } |
There was a problem hiding this comment.
addRealmMapping currently only registers the scoped prefix into importMap and the global prefixMappings, but it doesn't interact with urlMappings (so it doesn't affect mapURL()/handle() remapping). The name makes it easy to assume it replaces addURLMapping for URL-based realm mapping too; consider either documenting this limitation explicitly on the method or extending it to handle URL-like realmIdentifier inputs by delegating to addURLMapping where appropriate.
| assert.true( | ||
| realms.includes('@test/realm/' as any), | ||
| 'contains the registered realm', | ||
| ); |
There was a problem hiding this comment.
These assertions use as any to satisfy the RealmIdentifier[] type, which disables type checking in the test. Since RealmIdentifier is already imported in this file, prefer casting the string literal to RealmIdentifier (or otherwise comparing as string) so the test stays type-safe.
| assert.strictEqual(realms.length, 2); | ||
| assert.true(realms.includes('@test/realm/' as any)); | ||
| assert.true(realms.includes('@test/other/' as any)); | ||
| unregisterCardReferencePrefix('@test/other/'); |
There was a problem hiding this comment.
Same issue here: using as any defeats the purpose of the branded RealmIdentifier type and can hide mistakes. Prefer as RealmIdentifier (already imported) or cast the array to string[] before calling includes.
This is another step toward
@cardstack/base, more setup forRealmIdentifierandRealmResourceIdentifier:VirtualNetwork#addRealmMappingto encompassaddURLMapping,addImportMap, andregisterCardReferencePrefixhost/app/services/networkmain.tsandworker.tsstartup handlersVirtualNetwork#fetchVirtualNetwork.knownRealmsas a means to access the registered realmsThe only changes to existing code are the subbullets, the next phase will be more significant. There’s a new test that shows that
@cardstack/base/card-apiresolves properly, but no code will actually be trying to do that yet.