Skip to content

chore: resolve ember-provide-consume-context to one copy for base and host - #6183

Merged
backspace merged 3 commits into
mainfrom
align-provide-consume-context
Sep 18, 2026
Merged

backspace merged 3 commits into
mainfrom
align-provide-consume-context

Conversation

@backspace

@backspace backspace commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Split out of #6087 so it can be reviewed on its own.

packages/base declared ember-provide-consume-context at ^0.7.0 and packages/host at ^0.8.0, so pnpm installs two copies. Context is looked up by the identity of the provider's class, so a consumer from one copy never finds a provider from the other.

Nothing consumes base's copy today: base modules reach the host at runtime through the virtual network's shim, which serves the host's copy. The duplicate only bites when something compiles base's source against base's own dependencies — which is what serving @cardstack/base from the host bundle does, and where this was found: a card consumed context from 0.7.1 while the host provided it from 0.8.0, and edit controls stopped rendering.

Aligning the range to ^0.8.0 collapses both onto one copy.

Two follow-ons, both from review

local-types was still on ^0.7.0 and was the only reason 0.7.1 stayed in the lockfile at all. Nothing under that package references the module, so it is bumped too and the resolution is now gone from the tree — which is what stops the next manifest copied from a neighbour picking 0.7 back up.

ember-modifier was the same split, one package over. base declared ^3.2.1 while every other package declares ^4.1.0, and base avoided a second copy only because the overrides entry in pnpm-workspace.yaml rewrites it: the lockfile records base's specifier as ^4.1.0 resolving to 4.3.0, not what the manifest claimed. base was compiling against a major version its own manifest disagreed with, on a dependency shimmed through the same path that made the context bug reachable. It now declares what is actually resolved, which costs nothing at install time and removes the dependence on the override.

I originally wrote that this was the only split range among the 38 deps base and host share. That was wrong — ember-modifier is a second one, and it is the one that mattered. @glimmer/tracking differs too (^1.0.4 vs ^1.1.2) but both resolve to 1.1.2.

Single-homed in the catalog

Five packages hardcoded the range — base, host, catalog, boxel-cli, local-types — which is the drift being repaired here with nothing to stop it recurring. ember-provide-consume-context: ^0.8.0 now lives in pnpm-workspace.yaml's catalog and all five declarations point at it.

Every importer records specifier: 'catalog:' resolving to 0.8.0, 0.7.1 is absent from the lockfile, and pnpm install --frozen-lockfile succeeds. packages/boxel-cli is published to npm and already declares 48 of its dependencies this way, and the catalog value matches what it declared before, so the published manifest is unchanged. Touching that package is why the title carries a chore: prefix — no version bump, since nothing it ships moves.

On the lockfile diff

It adds bufferutil / supports-color / utf-8-validate to transitivePeerDependencies on four @percy/cli-* entries under snapshots:. That is pnpm-version churn rather than a resolution change: regenerating from main's lockfile with only these manifest edits reproduces the same lines, so reverting them would only be undone by the next install. pnpm install --lockfile-only on this branch produces no further change.

🤖 Generated with Claude Code

base asked for ^0.7.0 and host for ^0.8.0, so pnpm installed both and the
two packages resolved to different instances. That was invisible while
base modules were fetched, because the loader shimmed the package and
handed card code the host's instance. Bundling makes the bundler resolve
base's own import instead, so a card consumed context from 0.7.1 while
the host provided it from 0.8.0 and the two registries never met.

The symptom is a card seeing none of the context the host provides:
`@context.searchResultsComponent` is undefined, so a card rendering
search results prerenders an empty placeholder.

Aligning base's peer range deduplicates the install. No other shared
dependency resolves differently between the two packages.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 17, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-17T20:30:04.546586Z 7c7c1f8 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Preview deployments

Host Test Results

    1 files      1 suites   2h 37m 9s ⏱️
4 914 tests 4 900 ✅ 14 💤 0 ❌
4 929 runs  4 915 ✅ 14 💤 0 ❌

Results for commit e92c6bd.

Realm Server Test Results

    1 files    244 suites   1h 31m 11s ⏱️
3 618 tests 3 618 ✅ 0 💤 0 ❌
3 669 runs  3 669 ✅ 0 💤 0 ❌

Results for commit e92c6bd.

@backspace
backspace requested a review from a team September 18, 2026 09:21

@habdelra habdelra left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[Claude Code 🤖] Reviewed the resolution claim itself: whether the range change actually collapses the copies, what else in the tree still pulls 0.7.1, and whether the same split hides elsewhere among the deps base shares with the host through the externals.ts shim. I didn't exercise a build — CI's host and realm suites cover that.

The change is correct and does what it claims for base and host — no blocking issues. base's peer now resolves to 0.8.0 in the same importer entry the host uses, and host is base's only consumer, so the peer range is satisfied. Three non-blocking asks, all about what the change leaves behind.

  1. packages/local-types still declares ^0.7.0, and it's now the sole reason 0.7.1 remains in the lockfile at all. Nothing under that package references the module — grep across its .d.ts files (index.d.ts, eslint-js.d.ts, marked-gfm-heading-id.d.ts, matrix-js-sdk/) comes back empty — so is that declaration load-bearing for something I'm not seeing, or can it be dropped (or moved to ^0.8.0)? Either way removes the old copy from the tree, so the next manifest that copies from a neighbour can't pick 0.7 back up.

  2. ember-modifier in base's manifest — see the file comment on packages/base/package.json. Same class, currently masked by a workspace override rather than fixed, and it's shimmed through the same virtual-network path.

  3. Single-home the range via catalog: — see the inline comment on the changed line.

Also: the lockfile diff adds bufferutil / supports-color / utf-8-validate to transitivePeerDependencies on four @percy/cli-* entries under snapshots:. That's unrelated to this change and has no effect on resolution (the section is informational), so it reads as pnpm-version churn. Reverting those hunks would keep the lockfile diff to the one package it's about; leaving them is fine too, but worth a word so the next reader doesn't hunt for the connection.


Generated by Claude Code

Comment thread packages/base/package.json Outdated
Comment thread packages/base/package.json
backspace and others added 2 commits September 18, 2026 15:50
`local-types` was the only remaining `^0.7.0` declaration and nothing
under it references the module, so it was the sole reason 0.7.1 stayed in
the tree. Bumping it removes that resolution entirely, which is what stops
the next manifest copied from a neighbour picking 0.7 back up.

`ember-modifier` is the same split, one package over: base declared
`^3.2.1` while every other package declares `^4.1.0`, and base avoided a
second copy only because the `overrides` entry rewrites it — the lockfile
records base's specifier as `^4.1.0` resolving to 4.3.0, not what the
manifest claimed. So base compiled against a major version its own
manifest disagreed with, on a dependency shimmed through the same path
that made the context bug reachable. Declaring what is actually resolved
costs nothing at install time and removes the dependence on the override.

The lockfile keeps the `bufferutil` / `supports-color` / `utf-8-validate`
lines added to four `@percy/cli-*` `transitivePeerDependencies` entries.
They are pnpm-version churn, not a resolution change: regenerating from
main's lockfile with only these manifest edits reproduces them, so
reverting them would only be undone by the next install.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Five packages hardcoded the range, which is the drift this branch repairs
with nothing to stop it recurring. `pnpm-workspace.yaml`'s catalog is the
repo's one home for that and already carries most of the `ember-*` deps,
so the version lives there and all five declarations point at it. The next
bump cannot leave one behind.

Every importer now records `specifier: 'catalog:'` resolving to 0.8.0, and
0.7.1 is absent from the lockfile. `packages/boxel-cli` is published to
npm and already declares 48 of its dependencies this way, so the form is
established there; the catalog value matches what it declared before, so
the published manifest is unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@backspace backspace changed the title Resolve ember-provide-consume-context to one copy for base and host chore: resolve ember-provide-consume-context to one copy for base and host Sep 18, 2026
@backspace
backspace merged commit 9c2ad5a into main Sep 18, 2026
80 checks passed
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