Skip to content

Change relativeTo parameter to accept RRIs#4931

Open
backspace wants to merge 1 commit into
mainfrom
rri-widen-relativeTo-cs-10966
Open

Change relativeTo parameter to accept RRIs#4931
backspace wants to merge 1 commit into
mainfrom
rri-widen-relativeTo-cs-10966

Conversation

@backspace
Copy link
Copy Markdown
Contributor

The intermediate goal here on the way to @cardstack/base is to remove resolveCardReference, prefix mappings, cardIdToURL. This is a step toward that, where relativeTo would then no longer accept URLs.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 21, 2026

Preview deployments

Host Test Results

    1 files      1 suites   1h 49m 7s ⏱️
2 724 tests 2 709 ✅ 15 💤 0 ❌
2 743 runs  2 728 ✅ 15 💤 0 ❌

Results for commit 4cd3f13.

Realm Server Test Results

    1 files  ±    0      1 suites  +1   10m 17s ⏱️ + 10m 17s
1 482 tests +1 482  1 482 ✅ +1 482  0 💤 ±0  0 ❌ ±0 
1 573 runs  +1 573  1 573 ✅ +1 573  0 💤 ±0  0 ❌ ±0 

Results for commit 4cd3f13. ± Comparison against earlier commit ab1f1a9.

Widens the BaseDef [relativeTo] symbol field and the cluster of related
serialization-plumbing parameters from `URL | undefined` to
`RealmResourceIdentifier | URL | undefined`. Most call sites previously
wrapped raw RRIs in `cardIdToURL(...)` to coerce to URL form; with the
parameter types now accepting RRI directly, those wrappers are dropped
and the prefix-form / URL-form identifier flows through deserialization
unchanged.

The `.href` consumer sites in rich-markdown.gts and asset.gts handle
both forms — branching on `typeof x === 'string'` to coerce via
cardIdToURL only when needed.

# store.ts:1862 — normalize non-resolvable instance ids

Found via a matrix bisect (see CS-10966 PR for the full diagnostic
methodology): a server-returned doc whose `data.id` is a bare local id
(e.g. welcome-to-boxel.json's hardcoded `"welcome-to-boxel-sample"`)
previously survived only because `cardIdToURL(doc.data.id!)` would throw
on it, getting caught upstream and turned into a `CardErrorJSONAPI`. With
the cardIdToURL wrapper removed, the bare id would slip into
`instance.id` and later collide with the canonical URL form during
re-deserialization, surfacing as a "cannot change the id for saved
instance" realm-server 500.

The fix extends the existing "missing id" normalization at store.ts to
also catch non-resolvable ids, replacing them with the canonical URL
form derived from the fetch URL.

CS-10966.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@backspace backspace force-pushed the rri-widen-relativeTo-cs-10966 branch from ab1f1a9 to 4cd3f13 Compare May 22, 2026 18:40
@backspace backspace marked this pull request as ready for review May 22, 2026 19:25
@backspace backspace requested a review from a team May 22, 2026 20:04
@habdelra habdelra requested a review from Copilot May 22, 2026 21:55
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR advances the migration away from URL-only resolution by widening relativeTo to accept RealmResourceIdentifier (RRI) strings across runtime-common, base, and host codepaths, reducing reliance on cardIdToURL while keeping resolution behavior correct where new URL() is still required.

Changes:

  • Widen relativeTo parameter types across serializers, card API, commands, and host store flows to accept RealmResourceIdentifier | URL.
  • Update key call sites to pass RRIs (or raw id strings) instead of eagerly converting to URL via cardIdToURL.
  • Add host-side normalization to prevent non-resolvable/bare data.id values from being assigned to instances and later colliding with canonical URL IDs.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/runtime-common/serializers/index.ts Updates serializer interface to accept RealmResourceIdentifier in deserialize context.
packages/runtime-common/serializers/code-ref.ts Widens relativeTo typing for code-ref deserialization/adjustments toward RRIs.
packages/runtime-common/realm-index-query-engine.ts Switches relativeTo derivation for query-field population to use RRIs.
packages/runtime-common/index.ts Widens public API types (create/choose/internalKeyFor) to accept RRI relativeTo.
packages/runtime-common/commands.ts Widens command naming helper’s relativeTo to accept RRIs.
packages/host/app/services/store.ts Passes RRIs through patched relationship loading; normalizes inbound doc IDs to avoid ID collisions.
packages/host/app/routes/render.ts Passes RRI relativeTo into store.add during hydration.
packages/host/app/components/operator-mode/create-file-modal.gts Stops coercing spec IDs to URL objects; passes ID/RRI as relativeTo.
packages/experiments-realm/asset.gts Safely converts string relativeTo/id to URL only when needed for new URL().
packages/base/spec.gts Passes spec card IDs directly as relativeTo when loading definitions.
packages/base/rich-markdown.gts Handles relativeTo as either string (RRI) or URL when computing base URLs.
packages/base/card-serialization.ts Treats model relativeTo as `RealmResourceIdentifier
packages/base/card-api.gts Widens relativeTo/field deserialize plumbing and stores RRIs in [relativeTo] where appropriate.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 51 to 56
export async function deserialize<T extends BaseDefConstructor>(
this: T,
codeRef: ResolvedCodeRef | {},
_relativeTo: URL | undefined,
_relativeTo: RealmResourceIdentifier | URL | undefined,
): Promise<BaseInstanceType<T>> {
return { ...codeRef } as BaseInstanceType<T>; // return a new object so that the model cannot be mutated from the outside
// the bare id would be assigned to `instance.id` and later
// collide with the canonical URL form during re-deserialization
// (card-api.gts's "cannot change the id" guard).
json.data.id = url as RealmResourceIdentifier;
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