kuroshiro icon indicating copy to clipboard operation
kuroshiro copied to clipboard

async conversion should be sync

Open iliakan opened this issue 5 years ago • 8 comments

Right now, there's totally no point to make convert call async.

More than that, all modules that do similar stuff (limax, slug, slugify) do the conversion synchronously.

Initialization stuff can be async - all right, but the conversion? That makes kuroshiru unusable in many situations where sync method is required.

I looked into the code of https://github.com/hexenq/kuroshiro-analyzer-kuromoji, the conversion may be sync for sure.

Right now I can't use kuroshiro, because another module that needs the slug is 3rd-party and it expects a sync call. Not await. And that's perfectly normal, because all similar modules are sync.

iliakan avatar Oct 19 '18 18:10 iliakan

Hello iliakan,

Admittedly like what you said, kuroshiro-analyzer-kuromoji might be able to be sync. However, when it comes to kuroshiro-analyzer-mecab and kuroshiro-analyzer-yahoo-webapi , web or device I/O make them hard to be sync. To reach a compromise and offer a unified explicitly api, Kuroshiro uses async/await now.

See if we can find some workarounds.

hexenq avatar Nov 20 '18 08:11 hexenq

Right now all similar modules for other languages (including Chinese) use sync API. That's kind of a standard.

So people who add Japanese to their website, like me, get surprised and upset, because they can't use async API (all enclosing functions are sync, hard to rewrite, especially 3rd-party stuff).

If you'd want to make your tool more powerful and it requires async API - fine, but please make it optional.

The main API that most people will use should be sync, as expected.

iliakan avatar Nov 20 '18 09:11 iliakan

Thanks for the explanation. I understand the pain point. Could you name some of the modules you mentioned? I'd like to look a little deeper first.

hexenq avatar Nov 20 '18 12:11 hexenq

For instance, https://www.npmjs.com/package/slugify, a module with 149k weekly downloads, or this one with 198k https://www.npmjs.com/package/slug. These seem pretty popular.

And other modules labelled slug https://www.npmjs.com/search?q=keywords:slug.

iliakan avatar Nov 20 '18 12:11 iliakan

That's for Chinese (includes dictionaries I suppose) https://github.com/hotoo/pinyin

iliakan avatar Nov 20 '18 12:11 iliakan

Same issue here. Using kuroshiro-analyzer-kuromoji should have an option for a synchronous option. Very difficult to have a reactive search result ATM

micky2be avatar May 20 '20 08:05 micky2be

I had a look into turning kuroshiro sync, and it seems that the callback on kuromoji (via using kuroshiro-analyzer-kuromojii) has a slight delay when loading the dictionary files during building which meant sometimes between the time I initialise the analyser and then do the first parse, it threw an error due to the analyser not being loaded.

At the moment I have kuroshiro.convert() and kuroshiroAnalyzer.parse(), except leaving kuroshiro.init() and kuroshiroAnalyzer.init() as sync, which is probably necessary since @hexenq mentioned the other analysers need to be async as well:

  • https://github.com/lvl99/kuroshiro/tree/change-to-sync
  • https://github.com/lvl99/kuroshiro-analyzer-kuromoji/tree/change-to-sync

For the moment I've done the work purely for feasibility -- I wouldn't advocate for what I've done to be merged into the existing library, but I'd hope for maybe a convertSync and parseSync option to allow for the existing async convert/parse to not break existing systems.

lvl99 avatar Jun 11 '20 13:06 lvl99

If this was sync, I could use it properly in Vue.

sudofox avatar Jun 02 '21 20:06 sudofox