flexsearch icon indicating copy to clipboard operation
flexsearch copied to clipboard

Feature Request: Revise index.export() to return an Array of Promises

Open WesleyMConner opened this issue 2 years ago • 8 comments

Currently

  • The function index.export( function(key, data) {..} ), returns a single value - the boolean true.
  • The registered callback function is invoked four times, once each for keys reg, cfg, map and ctx.

Requested Improvement

-Adjust index.export() to return an array of promises - i.e., one promise for each prospective callback invocation. [Alternately, return an iterable suitable for a Promise.All() construct.]

Benefits

  • The proposed would facilitate tracking (e.g., blocking on) the resulting N callbacks.
  • Going forward, Flexsearch could adjust the number of callback invocations with less (if any) client impact.
  • Errors (rejections) could be handled within the Promise framework.

Example

Simulate an exception in the callback function.

myErrantCallback (k, v) {
  // Convert k to kx for https://github.com/nextapps-de/flexsearch/issues/273
  let kx = k;
  if (k.lastIndexOf('.') > 0) {
    kx = k.substring(k.lastIndexOf('.') + 1);
  }
  // Throw a random exception 15% of the time.
  if (Math.floor(Math.random() * 100) < 15) {
    throw new Error(`k: ${k}, kx: ${kx}, v: ${v}`);
  } else {
    console.log(`k: ${k}, kx: ${kx}, v: ${v}`);
  }
}

The current export() facility, requires clients to anticipate and track the number of callback invocations AND deal with any exceptions that arise. The proposed facility simplifies these tasks.

Promise.All(index.improvedExport( function(key, data) {..} )
  .then(...)     // Take an action after all N Promises are fulfilled.
  .catch(...)    // Centrally manage exceptions.

WesleyMConner avatar Oct 06 '21 16:10 WesleyMConner

If I had this, I could do something like:

const bits = await Promise.all(index.export)
await fs.writeFile(bits, JSON.stringify(bits))

peterbe avatar Oct 18 '21 11:10 peterbe

#273 #274 #290 #299

function exportIndex(flexSearchIndex) {
  // https://github.com/nextapps-de/flexsearch/issues/299
  // https://github.com/nextapps-de/flexsearch/issues/274
  return new Promise((res, rej) => {
    let pkg = [];
    const expected = new Set([
      "reg",
      "reg.cfg",
      "reg.cfg.map",
      "reg.cfg.map.ctx"
    ]);
    const received = new Set();

    const setsEq = (a, b) => {
      if (a.size != b.size) {
        return false;
      }

      return Array.from(a).every(el => b.has(el));
    };

    flexSearchIndex.export((key, data) => {
      // https://github.com/nextapps-de/flexsearch/issues/290
      // https://github.com/nextapps-de/flexsearch/issues/273
      pkg.push([
        key
          .toString()
          .split(".")
          .pop(),
        data
      ]);
      received.add(key);

      if (setsEq(expected, received)) {
        res(JSON.stringify(pkg));
      }
    });
  });
}

rpsirois avatar Apr 04 '22 21:04 rpsirois

I don't need you to do anything. I just want to know how to deal with the data exported.

mmm8955405 avatar Apr 09 '22 14:04 mmm8955405

If you return a promise, you limit the grammar. It's very painful for people who don't need grammar candy. How should we handle the exported and imported data. I found that there is still undefined content. Why can't we use nodejs streaming?

mmm8955405 avatar Apr 09 '22 14:04 mmm8955405

We're working on a solution internally at my company. Someone will pull request when it makes it on the top of the to-do stack. In the short term this worked for me.

What do you mean by grammar? Ideally the function would return a promise but also support callback style (well, without it being called for each key separately).

rpsirois avatar Apr 09 '22 20:04 rpsirois

@rpsirois thanks for sharing the function for exporting an index, it is very useful. I just wanted to double check if you intended to release it under the same license (Apache 2.0) as flexsearch?

jdddog avatar Apr 28 '22 01:04 jdddog

There's no license it's just some random code posted on the internet. If we do a pull request it will fall under the FlexSearch license since the code will be incorporated into their project.

rpsirois avatar Apr 28 '22 02:04 rpsirois

I don't think that export() needs to return an array of promises. If you need an array of promises you can just create one and add the created promises in the callbacks, as seen in the example provided by @rpsirois . But you probably never need this.

What doesn't make sense is that export() is an asynchronous function by nature that invokes a callback and yet it doesn't provide any way to know when it's done invoking callbacks for all keys. So users need to jump through hoops and compare the keys provided to callbacks with some list of possible keys.

What's needed is that export() should return a single promise when it's done, i.e. finished calling all callbacks, and, if needed, waiting for callbacks that return promises. Then resolve.

So you can simply write:

await index.export();

By the way, the Typescript definitions incorrectly (for the actual code) type export() as returning Promise<void>, which was very confusing.

danielvy avatar Jun 02 '22 18:06 danielvy

This is now fixed in v0.7.23 Further improvements to provide Promise.all() compatible export ist coming in next version.

ts-thomas avatar Oct 03 '22 12:10 ts-thomas

This is great news!

I don't see a v0.7.23 tag or npmjs.com release.

What's the recommended way to pull and benefit from the latest stable code?

WesleyMConner avatar Oct 03 '22 13:10 WesleyMConner

@ts-thomas Thanks for all your hard work on this!

rpsirois avatar Oct 03 '22 16:10 rpsirois

I see 0.7.3 and 0.7.31 now. Perfect timing as I'm in this code today.

Very much appreciated!

WesleyMConner avatar Oct 03 '22 17:10 WesleyMConner