Remove Context service definition stack capture - #7084
Conversation
🦋 Changeset detectedLatest commit: 91ad47f The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — please add a changeset for this exported API change.
Reviewed changes
packages/effect/src/Context.ts: removed definition-location stack capture fromContext.Service, dropped the optionalstackproperty fromContext.Key, removedstackfromService.toJSON(), and removed the "(defined at ...)" suffix from missing-service errors.packages/effect/test/Context.test.ts: added a regression test asserting services no longer expose astackproperty and thattoJSON()excludes it.
ℹ️ Missing changeset
This PR changes runtime behavior and an exported public type (Context.Key.stack removed; Service.toJSON() shape changed), so it should include a .changeset/*.md entry per the repository convention.
Technical details
# Missing changeset for Context service stack removal
## Affected sites
- `packages/effect/src/Context.ts` — `Key` interface no longer has `stack`; `Service.toJSON()` no longer emits `stack`; missing-service errors no longer append "(defined at ...)".
## Required outcome
- A changeset file describing the removal of `Context.Key.stack` and the new JSON/error-message shape is present in `.changeset/`.
## Suggested approach
- Add a `.changeset/remove-context-service-stack.md` (or similar) with:
```md
---
"effect": patch
---
Remove the optional `stack` field from `Context.Key` and `Service.toJSON()`, and stop appending definition locations to missing-service errors.
```@v0 or keep the SHA fresh with Dependabot ｜ Fix it ➔ ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
Important
The latest commit removed the regression test that verified the stack-capture removal, and the PR still lacks a changeset for this exported API/runtime change.
Reviewed changes
Since the prior pullfrog review, commit d0c103a92 removed the regression test that the previous commit had added. No changeset was added for the public Context.Key.stack removal and Service.toJSON() shape change.
⚠️ Missing changeset
This PR removes the optional stack property from the public Context.Key interface and changes the shape of Service.toJSON(). These are exported API/runtime changes that require a .changeset/*.md entry per repository convention. The changeset-bot also flagged the absence.
Technical details
# Missing changeset for Context service stack removal
## Affected sites
- `packages/effect/src/Context.ts` — `Key` interface no longer has `stack`; `Service.toJSON()` no longer emits `stack`; missing-service errors no longer append "(defined at ...)".
## Required outcome
- A changeset file describing the removal of `Context.Key.stack` and the new JSON/error-message shape is present in `.changeset/`.
## Suggested approach
- Add `.changeset/remove-context-service-stack.md` (or similar) with:
```md
---
"effect": patch
---
Remove the optional `stack` field from `Context.Key` and `Service.toJSON()`, and stop appending definition locations to missing-service errors.
```⚠️ Regression test removed without replacement
Commit d0c103a92 deleted the only test that verified services no longer expose a stack property and that Service.toJSON() excludes it. Without this coverage, the primary behavioral change of the PR has no automated regression guard.
Technical details
# Missing regression test for Context service stack removal
## Affected sites
- `packages/effect/test/Context.test.ts` — the "does not capture the service definition location" test added by `e668bb3d8` was removed by `d0c103a92`.
## Required outcome
- A test exists that asserts `"stack" in Service` is `false` and that `Service.toJSON()` returns `{ _id: "Service", key: ... }` without a `stack` field.
## Suggested approach
- Restore the removed test (or an equivalent assertion) in `packages/effect/test/Context.test.ts`.@v0 or keep the SHA fresh with Dependabot ｜ Fix it ➔ ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
Important
The changeset requested in the prior review is now present, but the regression test removed in d0c103a92 still has not been restored.
Reviewed changes
Since the prior pullfrog review, commit 91ad47f13 added .changeset/calm-services-rest.md to document the removal of definition-location stack capture from Context.Service.
⚠️ Regression test still missing
Commit d0c103a92 removed the only test that verified services no longer expose a stack property and that Service.toJSON() excludes it. The latest commit does not restore that coverage. Without an automated guard, the primary behavioral change of this PR has no regression protection.
Technical details
# Missing regression test for Context service stack removal
## Affected sites
- `packages/effect/test/Context.test.ts` — the "does not capture the service definition location" test added by `e668bb3d8` was removed by `d0c103a92` and has not been restored.
## Required outcome
- A test exists that asserts `"stack" in Service` is `false` and that `Service.toJSON()` returns `{ _id: "Service", key: ... }` without a `stack` field.
## Suggested approach
- Restore the removed test (or an equivalent assertion) in `packages/effect/test/Context.test.ts`.@v0 or keep the SHA fresh with Dependabot ｜ Fix it ➔ ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|
1 participant
Summary
Context.Servicefrom capturing definition-location stack framesTesting
nix develop -c pnpm --dir packages/effect checknix develop -c pnpm lintCloses EFF-525