Skip to content

fix(framework-components): call descriptor.factory as a method - #30222

Merged
SevInf merged 1 commit into
mainfrom
fix-materialize-codec-binding
Sep 7, 2026
Merged

fix(framework-components): call descriptor.factory as a method#30222
SevInf merged 1 commit into
mainfrom
fix-materialize-codec-binding

Conversation

@StevenMcClankerton

@StevenMcClankerton StevenMcClankerton commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Summary

materializeCodec evaluated descriptor.factory to a bare value before calling it, so this was undefined inside the factory. Every codec built as new XCodec(this) — the standard pattern for class-based codecs — received descriptor === undefined.

CodecImpl.get id() is this.descriptor.codecId, so codec.id threw on every such codec.

Why it went unnoticed

id is read almost nowhere: the decode and encode failure wrappers read it to build their message, and little else does. So the fault only surfaced on an already-failing path — and when it did, it replaced a diagnostic that names the column with one that names nothing:

TypeError: Cannot read properties of undefined (reading 'codecId')

The wrapper's real message — Failed to decode column <table>.<column> with codec '<id>' — already existed and never got the chance to render. Both decoding.ts and encoding.ts were affected.

The change

-  return blindCast<
-    (params: unknown) => (ctx: CodecInstanceContext) => Codec,
-    'registry erases P to any; paramsSchema validates input before forwarding'
-  >(descriptor.factory)(validated)(ctx);
+  return descriptor.factory(validated)(ctx);

The blindCast is not needed once the call is a method call: AnyCodecDescriptor is CodecDescriptor<any>, so the validated params pass without narrowing. Net one fewer cast against the ratchet.

Blast radius

this.descriptor is dereferenced at four production sites (codec.ts:73, the postgres and sqlite codec descriptors). None branches or memoises on it, so nothing depended on the undefined.

Closure-style factories were already correct — PostgresCodecDescriptorAdapter assigns this.factory = (params) => descriptor.factory(params), an arrow that forwards regardless of receiver. So adapted extension descriptors (pgvector, postgis, arktype-json) were unaffected; only directly-declared CodecDescriptorImpl subclasses were broken.

Testing

materialize-codec.test.ts covers a non-parameterized and a parameterized descriptor whose factories use new XCodec(this), and asserts codec.id. Verified load-bearing: against main's version 2 of the 3 cases fail with the exact Cannot read properties of undefined (reading 'codecId') above.

  • @internal/framework-components: 54 files, 628 tests
  • Codec consumers: @internal/sql-runtime 343, @internal/target-postgres 1596, @internal/adapter-postgres 867 + 3 expected-fail
  • Repo pnpm typecheck: 166/166; package lint clean

Provenance

Salvaged from #30195, which was closed. That PR paired this fix with an interim ::text[] projection cast for enum-array decoding; the cast is superseded by target-owned list framing, but this defect is unrelated to framing and is worth landing on its own.

🤖 Generated with Claude Code

https://claude.ai/code/session_01UcqoY3CKfnubdZt5YQk2Rq

Summary by CodeRabbit

  • Bug Fixes

    • Fixed codec creation so descriptor-bound factories retain the correct context.
    • Ensured both standard and parameterized codec references resolve correctly, including encoding and decoding behavior.
  • Tests

    • Added coverage for codec materialization with non-parameterized and parameterized codecs.

materializeCodec evaluated descriptor.factory to a bare value before
calling it, so `this` was undefined inside the factory. Every codec
built as `new XCodec(this)` therefore received `descriptor === undefined`,
and `codec.id` — which reads `this.descriptor.codecId` — threw.

The only readers of `codec.id` are the decode and encode failure
wrappers, so the fault surfaced as `Cannot read properties of undefined
(reading 'codecId')` from inside the code that builds the "failed to
decode column X.Y" message, replacing a diagnostic that names the
column with one that names nothing.

Calling factory as a method restores the receiver. The blindCast is no
longer needed: AnyCodecDescriptor is CodecDescriptor<any>, so the
validated params pass without narrowing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UcqoY3CKfnubdZt5YQk2Rq
Signed-off-by: Steven McClankerton <tatarintsev@prisma.io>
@StevenMcClankerton
StevenMcClankerton requested a review from a team as a code owner September 7, 2026 12:51
@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Team

Run ID: 128de15d-e1b4-4b7c-bcd1-b30d65d302ad

📥 Commits

Reviewing files that changed from the base of the PR and between dd846dc and 113ff6f.

📒 Files selected for processing (2)
  • packages/1-framework/1-core/framework-components/src/shared/resolve-codec.ts
  • packages/1-framework/1-core/framework-components/test/materialize-codec.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

materializeCodec now preserves the descriptor context when invoking codec factories. New tests cover non-parameterized and parameterized codec references, including encode and decode behavior.

Changes

Codec materialization

Layer / File(s) Summary
Bind descriptor factory invocation
packages/1-framework/1-core/framework-components/src/shared/resolve-codec.ts
materializeCodec invokes descriptor.factory with the descriptor as its receiver and passes validated parameters and instance context.
Validate materialized codecs
packages/1-framework/1-core/framework-components/test/materialize-codec.test.ts
Adds fixed and parameterized fixture descriptors. Tests verify codec identifiers and descriptor-bound encode/decode behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 113ff

Codec descriptor factories now retain their descriptor receiver during materialization, fixing class-based codec construction while preserving parameter and instance-context forwarding. Coverage includes fixed and parameterized codecs with encode/decode behavior, and no current merge-blocking risk remains.

Suggested reviewers: wmadden-electric

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: calling descriptor.factory as a method in framework-components.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-materialize-codec-binding

Comment @coderabbitai help to get the list of available commands.

@pkg-pr-new

pkg-pr-new Bot commented Sep 7, 2026

Copy link
Copy Markdown

Open in StackBlitz

@prisma/orm-extension-arktype-json

npm i https://pkg.pr.new/@prisma/orm-extension-arktype-json@30222

@prisma/orm-extension-middleware-cache

npm i https://pkg.pr.new/@prisma/orm-extension-middleware-cache@30222

@prisma/orm-extension-paradedb

npm i https://pkg.pr.new/@prisma/orm-extension-paradedb@30222

@prisma/orm-extension-pgvector

npm i https://pkg.pr.new/@prisma/orm-extension-pgvector@30222

@prisma/orm-extension-postgis

npm i https://pkg.pr.new/@prisma/orm-extension-postgis@30222

@prisma/orm-extension-supabase

npm i https://pkg.pr.new/@prisma/orm-extension-supabase@30222

@prisma/orm-family-mongo

npm i https://pkg.pr.new/@prisma/orm-family-mongo@30222

@prisma/orm-family-sql

npm i https://pkg.pr.new/@prisma/orm-family-sql@30222

@prisma/orm-framework

npm i https://pkg.pr.new/@prisma/orm-framework@30222

@prisma/orm-mongo

npm i https://pkg.pr.new/@prisma/orm-mongo@30222

@prisma/orm-postgres

npm i https://pkg.pr.new/@prisma/orm-postgres@30222

@prisma/orm-sqlite

npm i https://pkg.pr.new/@prisma/orm-sqlite@30222

@prisma/orm-target-mongo

npm i https://pkg.pr.new/@prisma/orm-target-mongo@30222

@prisma/orm-target-postgres

npm i https://pkg.pr.new/@prisma/orm-target-postgres@30222

@prisma/orm-target-sqlite

npm i https://pkg.pr.new/@prisma/orm-target-sqlite@30222

@prisma/orm-toolchain

npm i https://pkg.pr.new/@prisma/orm-toolchain@30222

commit: 113ff6f

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

size-limit report 📦

Path Size
postgres / no-emit 174.92 KB (-0.01% 🔽)
postgres / emit 152.11 KB (-0.01% 🔽)
mongo / no-emit 101.09 KB (-0.01% 🔽)
mongo / emit 90.95 KB (0%)
cf-worker / no-emit 198.88 KB (-0.01% 🔽)
cf-worker / emit 173.4 KB (-0.01% 🔽)

@SevInf
SevInf added this pull request to the merge queue Sep 7, 2026
Merged via the queue into main with commit 534e04f Sep 7, 2026
26 checks passed
@SevInf
SevInf deleted the fix-materialize-codec-binding branch September 7, 2026 13:37
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