refactor: insert and insertWithHooks functions are almost identical

[stearm]

Since the insert and the insertWithHooks are more or less the same functions, I'm wondering if it's worth adding a field inside the InsertConfig type like:

export type InsertConfig = {
  language: Language;
  enableAfterInsertHooks?: boolean;
};

and update the insert function like this:

export async function insert<S extends PropertiesSchema>(
  lyra: Lyra<S>,
  doc: ResolveSchema<S>,
  config?: InsertConfig,
): Promise<{ id: string }> {
  config = { language: lyra.defaultLanguage, ...config };
  const id = uniqueId();

  if (!SUPPORTED_LANGUAGES.includes(config.language)) {
    throw new Error(ERRORS.LANGUAGE_NOT_SUPPORTED(config.language));
  }

  if (!recursiveCheckDocSchema(doc, lyra.schema)) {
    throw new Error(ERRORS.INVALID_DOC_SCHEMA(lyra.schema, doc));
  }

  lyra.docs[id] = doc;
  recursiveTrieInsertion(lyra.index, lyra.nodes, doc, id, config, undefined, lyra.tokenizer as TokenizerConfigExec);
  trackInsertion(lyra);
  if (config?.enableAfterInsertHooks && lyra.hooks.afterInsert) {
    await hookRunner.call(lyra, lyra.hooks.afterInsert, id);
  }

  return { id };
}

Maybe it's too early to do this kind of refactoring but why not discuss it! 😄

[sundlemonflux]

Would be useful if hooks also got db instance itself for manipulation, just the document id is not overly useful as there isn't a way to access db inside the hook (as far as I know 😅 ).

[RafaelGSS]

That's expected. Hooks are still experimental, if we merge it into insert we would need to change its signature to return always a Promise, which is a breaking change.

We're evaluating a better approach and this likely will be refactored any time soon. By the way, would be great to see your use-cases using Hooks.

[micheleriva]

Closing this as Rafael's answer explains it 🙂