feat: update TypeScript support to 5.8 - 6.0, add typedoc for dialect packages#18209
Draft
WikiRik wants to merge 14 commits into
Draft
feat: update TypeScript support to 5.8 - 6.0, add typedoc for dialect packages#18209WikiRik wants to merge 14 commits into
WikiRik wants to merge 14 commits into
Conversation
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR updates TypeScript version targets to 6.0, bumps related dev dependencies, refactors several APIs from properties to getters for encapsulation, updates JSDoc references to use fully-qualified namespaces, expands TypeDoc configuration, and makes targeted improvements to database dialect implementations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
added 5 commits
April 12, 2026 17:43
WikiRik
changed the title
WikiRik
changed the title
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/postgres/src/connection-manager.ts (1)
393-407: Consider simplifying the redundant conditional checks.The
textFormat === 'text'conditionals on lines 395 and 403 are always true or the check is unnecessary sincetextFormatis either'text'orundefined, and both branches callgetTypeParserwith compatible arguments.♻️ Suggested simplification
if (typeData.type === 'range-array') { - return this.#buildArrayParser( - textFormat === 'text' - ? this.getTypeParser(typeData.rangeOid!, textFormat) - : this.getTypeParser(typeData.rangeOid!), - ); + return this.#buildArrayParser(this.getTypeParser(typeData.rangeOid!, 'text')); } if (typeData.type === 'array') { - return this.#buildArrayParser( - textFormat === 'text' - ? this.getTypeParser(typeData.baseOid!, textFormat) - : this.getTypeParser(typeData.baseOid!), - ); + return this.#buildArrayParser(this.getTypeParser(typeData.baseOid!, 'text')); }Since
#getCustomTypeParserreturnsnullfor binary format (line 381-383), this method only runs for text format. Always passing'text'explicitly is clearer and removes the conditional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/postgres/src/connection-manager.ts` around lines 393 - 407, The redundant ternary checks on textFormat in the array handling should be removed: since `#getCustomTypeParser` returns null for binary, this code only runs for text, so always call getTypeParser with the explicit 'text' argument and pass its result to `#buildArrayParser`; update both branches handling typeData.type === 'range-array' and typeData.type === 'array' to call `#buildArrayParser`(getTypeParser(typeData.rangeOid!,'text')) and `#buildArrayParser`(getTypeParser(typeData.baseOid!,'text')) respectively, eliminating the conditional and keeping existing symbols (`#buildArrayParser`, getTypeParser, typeData.rangeOid, typeData.baseOid, typeData.type).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Around line 104-107: The package.json currently pins "typescript": "6.0.2"
which is incompatible with the older ts-node tool; update package.json to remove
or replace ts-node usage by switching dev-dependency and any scripts referencing
"ts-node" to a TypeScript 6-compatible runner (for example replace the ts-node
devDependency with "tsx" and update scripts that call "ts-node" to call "tsx"
instead), or if you must keep ts-node add a clear comment and constrain
TypeScript to a compatible version (<6.0) and/or change scripts to use "ts-node
--swc" as a partial workaround; locate references to the package name "ts-node"
in package.json scripts and devDependencies and update them accordingly.
---
Nitpick comments:
In `@packages/postgres/src/connection-manager.ts`:
- Around line 393-407: The redundant ternary checks on textFormat in the array
handling should be removed: since `#getCustomTypeParser` returns null for binary,
this code only runs for text, so always call getTypeParser with the explicit
'text' argument and pass its result to `#buildArrayParser`; update both branches
handling typeData.type === 'range-array' and typeData.type === 'array' to call
`#buildArrayParser`(getTypeParser(typeData.rangeOid!,'text')) and
`#buildArrayParser`(getTypeParser(typeData.baseOid!,'text')) respectively,
eliminating the conditional and keeping existing symbols (`#buildArrayParser`,
getTypeParser, typeData.rangeOid, typeData.baseOid, typeData.type).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 632c1279-ec9a-48fd-805e-49839d899c2c
⛔ Files ignored due to path filters (2)
.yarn/patches/mariadb-npm-3.5.2-52603499c8.patchis excluded by!**/.yarn/**yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (43)
.github/workflows/ci.ymlpackage.jsonpackages/core/src/abstract-dialect/query-generator-typescript.tspackages/core/src/abstract-dialect/query-generator.jspackages/core/src/associations/base.tspackages/core/src/associations/belongs-to.tspackages/core/src/decorators/shared/model.tspackages/core/src/deferrable.tspackages/core/src/expression-builders/attribute.tspackages/core/src/expression-builders/cast.tspackages/core/src/expression-builders/col.tspackages/core/src/expression-builders/dialect-aware-fn.tspackages/core/src/expression-builders/fn.tspackages/core/src/expression-builders/identifier.tspackages/core/src/expression-builders/json-path.tspackages/core/src/expression-builders/json.tspackages/core/src/expression-builders/list.tspackages/core/src/expression-builders/literal.tspackages/core/src/expression-builders/random.tspackages/core/src/expression-builders/sql.tspackages/core/src/expression-builders/value.tspackages/core/src/expression-builders/where.tspackages/core/src/model-definition.tspackages/core/src/model.d.tspackages/core/src/model.jspackages/core/src/sequelize-typescript.tspackages/core/src/sequelize.d.tspackages/core/src/sequelize.internals.tspackages/core/src/sequelize.jspackages/core/test/integration/cls.test.tspackages/core/test/integration/data-types/data-types.test.tspackages/mariadb/package.jsonpackages/mariadb/src/_internal/data-types-db.tspackages/mariadb/src/connection-manager.tspackages/mariadb/src/query.jspackages/mssql/package.jsonpackages/mysql/src/_internal/connection-options.tspackages/postgres/src/_internal/connection-options.tspackages/postgres/src/connection-manager.tspackages/snowflake/src/dialect.tspackages/utils/src/common/get-synchronized-type-keys.tstypedoc.base.jsontypedoc.js
💤 Files with no reviewable changes (2)
- packages/core/src/decorators/shared/model.ts
- packages/core/src/model-definition.ts
added 2 commits
April 13, 2026 21:32
Member
Author
|
I've undone the suggestion by coderabbit to replace ts-node and will bring it back in another PR. This is large enough as it is |
ephys
reviewed
Apr 19, 2026
| "dayjs": "^1.11.20", | ||
| "lodash": "^4.18.1", | ||
| "mariadb": "^3.4.5", | ||
| "mariadb": "patch:mariadb@npm%3A3.5.2#~/.yarn/patches/mariadb-npm-3.5.2-52603499c8.patch", |
Member
There was a problem hiding this comment.
are we sure this is safe? It works in our installs, but does it work for our users? In all package managers?
Member
Author
There was a problem hiding this comment.
Good point! I forgot that we're shipping this together with the package for the user. This will break for non-yarn instances
Member
Author
There was a problem hiding this comment.
I've reverted the changes and pinned it to 3.4.5 until either the upstream fixes it or we deal with it another way.
EDIT: I've opened a PR to the upstream which should hopefully fix our issue.
Co-authored-by: Alyx <zoe@ephys.dev>
Drop the Yarn patch-based MariaDB dependency so published installs remain package-manager agnostic. Restore the pre-patch typings setup and pin 3.4.5 until the upstream type exports are fixed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Checklist
Description of Changes (generated by AI)
Summary
5.8,5.9, and6.0, and adjusted the codebase to work with newer dependency type surfaces.mariadb@3.5.2because its published typings are broken under this repo’snodenextTypeScript configuration.Details
TypeScript / CI
5.5-5.8to5.8-6.0.typescriptto6.0.2.@ts-ignoreworkarounds that are no longer needed with newer TypeScript versions.TypeDoc / documentation generation
typedocto0.28.19.typedoc-plugin-missing-exportsto4.1.3.sql.fn,sql.literal,ValidationError,TimeoutError,InferAttributes,InferCreationAttributes, and decorator helpers.@sequelize/coretargets so TypeDoc resolves them correctly.mysql2
enableCleartextPluginto the supported boolean connection options list.postgres
fallback_application_nameto the supported Postgres connection options.pg/pg-types.snowflake-sdk
browserRedirectPortto the supported Snowflake connection options.mariadb
mariadb@3.5.2declaration imports from./shareto./share.jsso TypeScript can resolve them undermoduleResolution: nodenext.Associations / constraint checking
Association.foreignKeyto a getter and updatedBelongsToAssociationto store it privately for better type/runtime consistency.ConstraintChecking.DEFERREDandConstraintChecking.IMMEDIATEso their class names are preserved correctly.Shared utility
getSynchronizedTypeKeysto string keys only, which matchesObject.keys(...)behavior and avoids newer dependency key-type mismatches.Notes
skipLibCheck.Summary by CodeRabbit
New Features
enableCleartextPlugin), PostgreSQL (fallback_application_name), and Snowflake (browserRedirectPort).Bug Fixes
Refactor
Association.foreignKeyfrom property to read-only getter.ConstraintChecking.DEFERREDandIMMEDIATEto static accessors.Dependencies
Documentation