Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

dep: upgrade pg-types to ^4 #3400

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aqeelat wants to merge 2 commits into brianc:master
base: master
Choose a base branch
Loading
from aqeelat:upgrade-types
Open

Conversation

Copy link

@aqeelat aqeelat commented Mar 9, 2025

When this pr is merged, we can update @types/pg to not depend on @types/pg-types

Copy link
Owner

brianc commented Mar 10, 2025

pg-types @ 4.x is not backwards compatible, so it requires a major version bump (and a LOT of headache for library consumers)

Copy link
Owner

brianc commented Mar 10, 2025

I think there needs to be another fork or something of pg-types that does not do the date/time zone conversion differently. Unfortunately changing that would be insanely breaking for a lot of deployed code-bases even though the original decision 15 years ago wasn't the best....I'm not convinced its worth the forced upgrade there.

Copy link
Author

aqeelat commented Mar 11, 2025

@brianc can you elaborate on what that means?

Also, does it make sense to use pg-types without node-postgres? If not, then why not move the source code for that library into this repo and tie their development together?

Copy link
Author

aqeelat commented Mar 11, 2025

Oh I just came across brianc/node-pg-types#50 and the PRs related to it.
Should I convert this into a discussion?

Copy link
Author

aqeelat commented Mar 11, 2025

From what I can gather:

  1. the Date objects will now be parsed to the current timezone even if they do not have timezone in them (which is why the test is failing). To fix this test, we could retrieve the timezone of the db connection and then add the offset.
  2. postgres DATEs will be returned as string

But yeah, this should be a major version bump

Copy link
Owner

brianc commented Mar 15, 2025

But yeah, this should be a major version bump

I am leaning towards re-absorbing pg-types in some form into this monorepo & before incorporating it, reverting the breaking changes to the date parsing. Then it can be more tightly versioned w/ the rest of the code, benefit from the years of perf tweaks done to the stand alone pg-types library, and not cause massive, hard to catch breaking changes like what would happen if i just bumped pg-types in this library.

Copy link
Contributor

abenhamdine commented Mar 22, 2025
edited
Loading

I am leaning towards re-absorbing pg-types in some form into this monorepo & before incorporating it, reverting the breaking changes to the date parsing. Then it can be more tightly versioned w/ the rest of the code, benefit from the years of perf tweaks done to the stand alone pg-types library, and not cause massive, hard to catch breaking changes like what would happen if i just bumped pg-types in this library.

That's the more reasonable move I think.
With a feature flag to enable the change on date parsing, so anyone could try it and start changing its code base for that.

Copy link
Author

aqeelat commented Apr 8, 2025

@brianc is there a anything I can do to help?

Copy link
Owner

brianc commented Apr 9, 2025

@aqeelat thanks for asking - just getting over the flu...I think the biggest thing that would help, though it's quite a bit of effort, is fork pg-types@4.x and then change it so the existing tests under ./packages/pg pass without any moidifications to the pg codebase. There are tests around the date timezone and date returning an actual date ( I believe) that are failing that need to pass. Basically make a pg-types@5.x that is not a breaking change. Then it'll be really easy to absorb.

Copy link
Author

aqeelat commented Apr 10, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /