Skip to content
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

feat (client): extensible value type for use with custom parsers #1791

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

kevin-dp
Copy link
Contributor

@kevin-dp kevin-dp commented Oct 3, 2024

This PR modifies the types of our client to enable users to extend the Value type. This is needed when using custom parsers.

For example, the parser below used to throw a type error because Date is not a Value (where Value is the type of base SQL values):

const parser: Parser = {
  timestampz: (date: string) => {
    return new Date(date)
  },
}

With this PR, we can now write this parser as:

const parser: Parser<Date> = {
  timestampz: (date: string) => {
    return new Date(date)
  },
}

And an example usage of this parser with the ShapeStream:

type CustomRow = {
  foo: number
  bar: boolean
  baz: string
  ts: Date
}

const shapeStream = new ShapeStream<CustomRow>({
  url: `...`,
  parser: {
    timestampz: (date: string) => {
      return new Date(date)
    },
  },
})

Note that this would throw an error as Date is not a type that occurs in the row:

type CustomRow = {
  foo: number
  bar: boolean
  baz: string
}

const shapeStream = new ShapeStream<CustomRow>({
  url: `...`,
  parser: {
    // type error here
    timestampz: (date: string) => {
      return new Date(date)
    },
  },
})

Copy link
Contributor

@KyleAMathews KyleAMathews left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah very nice! I noticed this and it bugged me and now there'll be a fix 💫

@balegas
Copy link
Contributor

balegas commented Oct 3, 2024

Thanks @kevin-dp, that will solve my problem.

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.

3 participants