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

added option to supply a custom from and to json converter function #233

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Manuelbaun
Copy link

What kind of change does this PR introduce?

It adds the option to supply a converter object with a from and to json converter function in the PostgrestBuilder-class.

What is the current behavior?

When fetching from the backend, the data gets parsed as regular JSON. This is fine, if no additionally processing is needed. But in case a replacer is needed in order to convert a ISO DateString to a Date Object. It is very handy to do that in the JSON.parse function call.

What is the new behavior?

With this change it is now possible to supply a convertor object, which is basically just a object with two props: fromJSON, toJSON

Example:

import { parseISO } from 'date-fns';

/// ...
client<MyType>('my_table')
.select('*')
.match({id: '1234'})
.withConverter({fromJSON: (k:string, v:any)=>{
      return ['start', 'end'].includes(k)? parseISO(v): v;
}
/// ...

Additional context

feel free to suggest another name.

@soedirgo
Copy link
Member

soedirgo commented Dec 14, 2021

Any reason this couldn't just be:

import { parseISO } from 'date-fns';

/// ...
client<MyType>('my_table')
.select('*')
.match({id: '1234'})
.then(res => { ...res, data: transformData(data) })
/// ...

?

@Manuelbaun
Copy link
Author

When I see it correctly, the then-method of the PostgrestBuilder-class already uses JSON.parse/JSON.stringify. If I do it like you suggested, then data would be already parsed and I had to walk the received object manually.

Lets assume, that is the structure/interface of MyType:

interface MyType {
    name: string;
    start: Date; // from the DB, we receive actually an string
    end: Date; // same here
    // some other important fields
    embeddedResource: {
        start: Date; // same here
        end: Date; // and here
        // some other important fields
        someOtherEmbeddedResource: {
            start: Date; // and here
            end: Date; // and here
            // some other important fields
        }[];
    }[];
}

So when JSON.parse parses the JSON, I could check the keys and convert the DateISO String, which I receive from postgrest, and parse it to date. Therefore I dont have to walk the structure by hand. The replacer does it already. So when I call then( ...., then the data start, end attributes would be parsed as a Date-Object.

@soedirgo
Copy link
Member

If you really have to, you can always do:

const transformData = data => data.map(row => JSON.parse(JSON.stringify(row), fromJSON))

I can see the value of using reviver/replacer (e.g. performance in the above case), but I think the use case is not general enough to warrant extending the API.

@Manuelbaun
Copy link
Author

Previously I used your posted suggestion, but the performance aspect lead to this pull request. parse -> stringify-> parse doesnt seem right, if it can happen in one go. Besides, the api would be optional.

But I also understand not to easily extending the API. If more from the comunity have use for this pull request, you could revisit it.

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.

None yet

2 participants