pocketbase / js-sdk

PocketBase JavaScript SDK
https://www.npmjs.com/package/pocketbase
MIT License
2.06k stars 122 forks source link

Question/Suggestion: Improving the SDK for constructing custom responses #282

Closed Odas0R closed 6 months ago

Odas0R commented 6 months ago

One thing that I've found a little hard to work with is the retrieval of expands, since when I do multiple expand, e.g

{
  expand: "x_via_y, x_via_y_via_z, ...
}

This results in an a response like

{
id
...
expand: {
  x_via_y: [
    {
      expand: {
        x_via_y_via_z: [
          ...
        ]
      }
    }
    ...
  ]
 }
}

Most of the time I'm dissecting the responses and it can get a little messy since I need a specific format for the data.

Is it something you would like to explore?

Would like to hear your feedback :)

Thanks, Guilherme

ganigeorgiev commented 6 months ago

I'm having difficulties understanding what feedback you are looking, nor what actually is the problem.

The structure of the response (including expand) is determined by the Web API and there are no plans for now to change it as it could break too many apps and, while nothing is set in stone before v1, there needs to be a good reason why it needs to change.

I'll close the issue for now as there is no actionable item here, but feel free to elaborate what you have in mind.

Odas0R commented 6 months ago

Okay, I'll try to be more brief.

Here's an example:

// NOTE: this code might be wrong, regarding the `expand` it's just for example purposes
async get(id: string): Promise<Entity> {
    const result = await pb.collection<EntityRecord>("entities").getOne(id, {
      expand: "entity_group_via_entity.group, assessments_via_entity",
    });

    const groups = expandAt<GroupRecord>(result, "entity_group_via_entity").map((g) => {
      return createEntityGroup(g.expand?.group);
    });

    const assessments = expandAt<AssessmentRecord>(result, "assessments_via_entity").map((assessment) => {
      const experts = expandAt<ExpertRecord>(assessment, "experts").map(createExpert);
      return {
        ...assessment,
        experts,
      };
    });

    return createEntity({
      ...result,
      groups,
      assessments,
    });
}

As you can see I need to dissect the response in order to construct the entity class under the createEntity method.

This is not ideal since I'm always mapping the response in order to organize the values into the data format I require.

It would be an improvement if we could build the response format as we want. Note that I don't have an specific API for this, nor I do not know what approach would be best. I'm asking if this could be something you would add.

Here's on top of my head a way to it:

const structure = {
    include: ['id', 'name'], // Top-level fields to include
    assessments: { // Relational nested structure example
        include: ['id', 'assessmentName'],
    },
    // this works as the `expand`, so one could take this "structure" and then
    // adapt to the current sdk model, or even construct the url from scratch.
    via_something_something: { // Another relational nested structure example
        key: "alternative_key",
        include: ['id', 'detail'],
        via_... : { 
          ...
        }
    },
};

// this would need to be adjusted to the pocketbase client, this is just an example
client.getWithCustomStructure('entities', 'entityId', structure)
    .then(transformedResponse => {
        console.log(transformedResponse);
    });

// console.log(transformedResponse)

{
  "id": "123",
  "name": "John Doe",
  "assessments": [
    {
      "id": "a1",
      "assessmentName": "Assessment 1"
    },
    {
      "id": "a2",
      "assessmentName": "Assessment 2"
    }
  ],
  // via_something_something
  "alternative_key": [
    {
      "id": "v1",
      "detail": "Detail 1"
      "via...": { ... }
    },
    {
      "id": "v2",
      "detail": "Detail 2"
      "via...": { ... }
    }
  ]
}

On a sidenote

I feel like the API of expand is a little ugly, specially the via_.... It would be much more maintainable if the actual expand were an object with specific properties that would classify the relations, but that's my opinion, just sharing feedback!

ganigeorgiev commented 6 months ago

The structure mapping feels way more complicated than what we do and I don't think it would be a good idea to have something like this builtin as I imagine it would be confusing for new users.

For now I remain against it, but if your specific use case has a need for this you can implement your own "transformers" by attaching to the pb.afterSend SDK hook. If there is enough user demand for such "transformers" support I may reconsider adding helper apis to facilitate it.

Odas0R commented 6 months ago

The structure mapping feels way more complicated than what we do and I don't think it would be a good idea to have something like this builtin as I imagine it would be confusing for new users.

For now I remain against it, but if your specific use case has a need for this you can implement your own "transformers" by attaching to the pb.afterSend SDK hook. If there is enough user demand for such "transformers" support I may reconsider adding helper apis to facilitate it.

Okay, understood.

How could I inject a custom transformer for each request? Seems that pb.afterSend is for a general usecase.

~I'll try to build a custom mapper, when built I'll share it here.~

Edit: I'll just keep the current expandAt method, since it got a little complex... Later on I'll try more things