Uh oh!
There was an error while loading. Please reload this page.
test(effect): Add Effect v3 compatibility tests for backwards compat - #20389
test(effect): Add Effect v3 compatibility tests for backwards compat#20389JPeer264 wants to merge 2 commits into
Conversation
size-limit report 📦
|
This adds support to Effect v4, but also keeps the compatibility for v3. There is no way that we can unit test against v3, as the `devDependencies` need to use `effect@4` and an updated `@effect/vitest` version, which is not compatible with Effect v3 (this is added in #20389). The API for Effect v4 has changed a little, so there are safeguards to detect if it is v3 or v4 and uses the correct API. The good part is that for users nothing changed, so they still can use the same methods in their app as before (ofc, respecting the new Effect v4 API). Before (Effect v3): ```ts const SentryLive = Layer.mergeAll( Sentry.effectLayer({ dsn: '__DSN__', tracesSampleRate: 1.0, enableLogs: true, }), Layer.setTracer(Sentry.SentryEffectTracer), Logger.replace(Logger.defaultLogger, Sentry.SentryEffectLogger), Sentry.SentryEffectMetricsLayer, ); ``` After (Effect v4): ```js const SentryLive = Layer.mergeAll( Sentry.effectLayer({ dsn: '__DSN__', tracesSampleRate: 1.0, enableLogs: true, }), Layer.succeed(Tracer.Tracer, Sentry.SentryEffectTracer), Logger.layer([Sentry.SentryEffectLogger]), Sentry.SentryEffectMetricsLayer, ); ``` Both usages still work and are represented in the E2E tests.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c328112. Configure here.
| '@sentry-internal/browser-integration-tests') }} | ||
| changed_effect_v3_compatibility: | ||
| ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, | ||
| '@sentry-internal/effect-3-compatibility-tests') }} |
There was a problem hiding this comment.
CI affected detection will never trigger for this package
Medium Severity
The changed_effect_v3_compatibility output uses contains(steps.checkForAffected.outputs.affected, '@sentry-internal/effect-3-compatibility-tests'), but this package is intentionally excluded from the yarn workspace (confirmed it's absent from the workspaces list in root package.json). Since NX only tracks workspace projects, @sentry-internal/effect-3-compatibility-tests will never appear in the affected list. On PRs, the job condition needs.job_build.outputs.changed_effect_v3_compatibility == 'true' will only be true when changed_ci is true (CI config changes), never when the test files themselves or @sentry/effect source code changes. This means the compatibility tests effectively won't run on most PRs.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit c328112. Configure here.
s1gr1d
left a comment
There was a problem hiding this comment.
Tests look fine, but I am not sure if we should have this as a top-level folder in the dev-packages. Maybe you can put it in an existing folder or create a new one version-compatibility-tests (or something more fitting for general library/framework tests)
nicohrubec
left a comment
There was a problem hiding this comment.
For other packages afaik the pattern if we support multiple versions is to keep one set of unit tests and test version compatibility via multiple e2e tests. I am wondering if this would also be sufficient here. Is there a particular reason you think this should be treated differently?
JPeer264
commented
Apr 21, 2026
I was also not very sure where to put it
We still have E2E tests, I wasn't sure if that is sufficient, but on a second thought E2E tests might do it as well - as I wanted the API to be typesafe and testable with the old I'll close this one for now and will revisit once there are any specific issues with not having unit tests. |
This adds Effect v3 compatibility tests as own folder. We can't really leave them inside
packages/effect, as there we need to have Effect v4 asdevDependency, so the old version can't be used nor tested anymore.Also the new
effect-3-compatibility-testsare not included in the yarn workspace. Here yarn somehow can't have two different effect versions installed and the tests just wouldn't work correctly (so I did an ownyarn.lockinside this folder to keep them separate. In case we would move over topnpmor something else we could try to add it as part of the workspace)I basically copied over everything inside
packages/effect/testinto this folder and changed thecreateMetricsFlusher()toyield* TestClock.adjust('10 seconds');, as this is more reliable and wouldn't rely on any internal testing functionality.