chore: cleanup deno support#6405
Conversation
|
fubhy
left a comment
There was a problem hiding this comment.
@lishaduck This looks good to me and also works for me locally. Just waiting for @tim-smart to confirm that the issue he was seeing before in the effect-smol PR are resolved.
Can you fix the linting and test errors in the meantime?
That was in the actual Deno PR, this PR is just cleaning up the support added in Effect-TS/effect-smol#80 with a newer Deno version (reiterating from Discord, the "next" version of Deno should simplify this even more and remove the need for Deno-specific checks; this is kinda prepping for that as well). |
|
And it looks like the test was just a timeout, fixed the formatting. |
|
@lishaduck Test is still timing out. |
They all time out for me locally on Node.js & Deno 😅 I'll see if an agent can repro it but I haven't a clue. |
Co-authored-by: Eli <elisha.dukes@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUpdates Deno and Vitest environment configuration, removes stack-trace type augmentations, and increases persisted-cache test timeouts. ChangesEnvironment configuration
Stack trace type cleanup
Persistence test timing
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/effect/test/unstable/persistence/PersistedCacheTest.ts`:
- Around line 69-71: In both pipelines in PersistedCacheTest.ts (anchor lines
69-71 and sibling lines 99-101), move flakyTest to be the outermost operator,
after Effect.catchFilter(...), so the 10-second timeout includes layer
provisioning while preserving the existing error handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 30dc34d1-abb9-4062-a50a-fee15e386b63
📒 Files selected for processing (9)
.vscode/settings.jsondeno.jsonpackages/effect/src/internal/stackTraceLimit.tspackages/effect/src/internal/tracer.tspackages/effect/src/unstable/reactivity/AtomHttpApi.tspackages/effect/src/unstable/reactivity/AtomRpc.tspackages/effect/test/StackTraceLimit.test.tspackages/effect/test/unstable/persistence/PersistedCacheTest.tsvitest.config.ts
💤 Files with no reviewable changes (3)
- packages/effect/src/unstable/reactivity/AtomRpc.ts
- packages/effect/src/internal/tracer.ts
- packages/effect/src/unstable/reactivity/AtomHttpApi.ts
Co-authored-by: Eli <elisha.dukes@gmail.com>
Bundle Size Analysis
|
Type
Description
Remove some Deno-specific code that's no longer necessary, etc.
Related
Port of the prepping from Effect-TS/effect-smol#859 to the main Effect repository.
Summary by CodeRabbit
New Features
Bug Fixes
Error.stackTraceLimitacross runtimes.ErrorConstructortype augmentations to reduce TypeScript conflicts.Tests