pocketbase / js-sdk

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

Purpose of wrapping returned object with `n` #105

Closed Fractal-Tess closed 1 year ago

Fractal-Tess commented 1 year ago

I've noticed this before, but I didn't bother to ask what it was for since I wrote some code to avoid it. Still, recently I started testing some of my models and I've run into the issue of needing to manually re-cast the object into proper form in order to do tests on it.

Here is an example

Suppose a simple pocketbase collection of just a title field as a string. Here are some types

//model/title.ts

export interface DatabaseAddons {
  collectionId: string;
  collectionName: string;
  updated: string;
  created: string;
  id: string;
  expand: {};
}

export type InsertableAltTitles = {
  title: string;
};
export type AltTitlesCollection = InsertableAltTitles & DatabaseAddons;

export const createAltTitle = async (data: InsertableAltTitles) => {
  try {
    return await getAltTitle(data.title);
  } catch (error) {
    if (error instanceof ClientResponseError)
      return await altTitleCollection.create<AltTitlesCollection>(data);
    logger.error(
      `Error while trying to create altTitle for ${data.title}`,
      error
    );
    process.exit(1);
  }
};

And here is the test file

const validator: { [key in keyof AltTitlesCollection]: Vi.ExpectStatic } = {
  id: expect.any(String),
  title: expect.any(String),

  collectionId: expect.any(String),
  collectionName: expect.any(String),

  created: expect.any(String),
  updated: expect.any(String),
  expand: expect.any(Object),
};

describe('Model cache', () => {
  const altTitle: InsertableAltTitles = { title: 'example title' };

  it('create new title', async () => {
    await expect(createAltTitle(altTitle)).resolves.toBe(validator);
  });
});

But this doesn't pass, because the returned object is wrapped in a n:{}

What is the purpose of this, and what is the preferred way of easily getting the data out of it?

ganigeorgiev commented 1 year ago

I'm not sure that I understand the example and what do you mean.

Could you post the exact error message and where is failing?

I guess the n is from the minification, but the name of the exported classes should be preserved.

Fractal-Tess commented 1 year ago

Here is a simple example

let name = 'example genre'

const result = await pb.collection('genre').getFirstListItem<GenreCollection>(
      `name='${name}'`
);

console.log(result)

Output:


n {
  collectionId: '99axzqout4huda7',
  collectionName: 'genre',
  created: '2022-12-09 07:32:19.789Z',
  id: 'qfmm36eqjqaa1b6',
  name: 'example genre',
  updated: '2022-12-09 07:32:19.789Z',
  expand: {}
}

Also, on a side note, is there an option to increase the timeout of the requests? I'm making a lot of them inside of an async pool of workers and most of them get dropped. Also, I'm not sure if this is expected, but the query time on a table of 1.5m records with a few columns is over 11 seconds for me. Are indexes supported?

My use case is that I'm trying to index some online media content - think of like an online book having x chapters, and for each one, I need to query to see if the chapter exists, and if it doesn't I need to make it. But each book may have 1000 chapters or more, so this is becoming very inefficient since I'm redoing this querying every x hours on a massive pool of books to see if any of them have new chapters or not. The database quickly fills up and I'm making 5m requests per hour, which is not something it can support as the speed becomes unbearably slow.

ganigeorgiev commented 1 year ago

I'm sorry, I still don't understand what is the issue here.

import PocketBase, { Record } from "pocketbase";

const pb = new PocketBase(...)

const record = pb.collection("examples").getOne("RECORD_ID");

console.log(record instanceof Record); // true
console.log(record.constructor.name); // some minified variable

We can adjust the mangling of the terser minification and enable the keep_classnames and keep_fnames props, but I don't see why this could cause an error in your case. It shouldn't really matter unless you are checking the constructor or function prototype name.


Also, on a side note, is there an option to increase the timeout of the requests? I'm making a lot of them inside of an async pool of workers and most of them get dropped.

Could you provide the exact error message?

The SDK doesn't really set any timeouts explicitly. The dropped requests could be either because of the autocancellation or of issue https://github.com/pocketbase/pocketbase/issues/1187 (later today there will be a v0.9.1 release with a fix for it).

Also, I'm not sure if this is expected, but the query time on a table of 1.5m records with a few columns is over 11 seconds for me. Are indexes supported?

Currently you can't set indexes from the Admin UI (see https://github.com/pocketbase/pocketbase/issues/345 and https://github.com/pocketbase/pocketbase/issues/544), but you could define index in a migration. With v0.9 we support JS migrations too:

// 1. Run ./pocketbase migrate create "new_index"
//
// 2. This will create a blank migration and you can add the following content:
migrate((db) => {
  db.newQuery(`CREATE INDEX your_index_name ON your_collection_name (field1, field2)`).execute();
}, (db) => {
  db.newQuery(`DROP INDEX IF EXISTS your_index_name`).execute();
})

I'm not familiar with the details of your project, but please keep in mind that we inherit some of the limitations of SQLite and if you rely on high concurrent writes PocketBase may not be suitable for your use case. For more info you could check https://www.sqlite.org/whentouse.html.

ganigeorgiev commented 1 year ago

@Fractal-Tess If the failing test is the toBe call and this is from the JEST suite then it is possible that your test is failing because JEST's toBe as stated in the documentation is using Object.is which, based on https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/is, considers 2 objects to be equal only if they reference the same object. For example this will not work:

var a = {"test": 1}
var b = {"test": 1}

Object.is(a, b) // false, even if they have the same props

It is also possible that it is expecting POJO (aka. plain object), but the SDK returns a custom Record object. You can try converting it to POJO by calling record.export() or use structuredClone(record)

Fractal-Tess commented 1 year ago

@Fractal-Tess If the failing test is the toBe call and this is from the JEST suite then it is possible that your test is failing because JEST's toBe as stated in the documentation is using Object.is which, based on https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/is, considers 2 objects to be equal only if they reference the same object. For example this will not work:

var a = {"test": 1}
var b = {"test": 1}

Object.is(a, b) // false, even if they have the same props

It is also possible that it is expecting POJO (aka. plain object), but the SDK returns a custom Record object. You can try converting it to POJO by calling record.export() or use structuredClone(record)

I see,

Thank you very much for the replay.

Perhaps this is even my bad since I'm not very experienced with the testing frameworks and might've missed this. I opted to use Zod to verify the shape, but I think I'll even get rid of that test.

Anyway, I was just wondering on why exactly the returned record is wrapped on this n:{} object like I mentioned above.

ganigeorgiev commented 1 year ago

n in this case is the Record class. It is not "wrapped", it's just not a POJO (Plain Old JavaScript Object). Without the terser minification this probably would look like Record: {}. But this shouldn't really matter for the end user because the exported n is aliased to Record (this is why the record instanceof Record is working).

Once again, we can enable the "real" class names but there shouldn't be a need for that unless you use record.constructor.name or Function.prototype.name to print the "real" class/function name.

I'll close the issue, but if you are still experiencing the error, please provide the actual error message and I'll investigate it in more details.