-
-
Notifications
You must be signed in to change notification settings - Fork 50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added JavaScript/wasm target via emscripten #171
Conversation
Is this change correct? RicBent@3f0eb0c Passing 0 for |
Attempted to implement the release workflow. Not sure how to test it properly without triggering a release. I guess a workflow that triggers on new pull requests still needs to be added. |
@RicBent |
Great! The latest commit adds a wrapper package that could be directly imported in any npm project. Along with proper types for the entirety of the so far exposed API (https://github.com/bab2min/Kiwi/blob/2161cf137b996471383e7c7b65370e9f28981f14/bindings/wasm/package/src/kiwi.ts). I'd be happy to implement the remaining API functionality to bring it to the same level as the Python library once you had a look at the PR. |
@RicBent |
I had a look at the Java bindings and it seems like the only missing API is the following: KiwiBuilder:
Kiwi:
I would also like to add documentation to the bindings. However I am not able to make them in Korean, only in English. Is that a problem @bab2min ? Oh and we would also need an npm package name as 'kiwi' is already taken. I chose 'kiwi-nlp' for now, but I can change it if you have a better suggestion. |
@RicBent |
Alright, I am almost there then. I already wrote pretty much all the required documentation and just the 4 points from I also amended the release workflow to automatically build and publish the resulting package to npm: Kiwi/.github/workflows/release.yml Lines 330 to 346 in c77e15f
The workflow requires To generate an npm token you need to do the following:
To add it to the repo's secrets you can follow this: |
@bab2min everything on my end should be done now:
Let me know if anything needs to be improved/changed! |
@RicBent |
@RicBent |
@ghj7211 I actually did the WASM port to use this inside a MV3 extension I built for my personal use. To make it work properly, also together with vite you need to following flags added to the current ones: When I made the PR I was unsure if there is a sane way to distribute different versions of the package and thus just went for the most general and minimal build flags for broadest compat and performance. Especially the dyn exection flag had a significant performance hit when I worked on something in the past. Only thing that I figured would be possible was multiple different published packages for different build flags, which I didn't bother to do considering the limited target audience and added complexity. If you do not want to build this package yourself, I could provide you with the build I am using. Not sure if you want to reply on pre-built, unverifiable binaries though. If you have any problems with your project, let me know. Chances are high that I had to deal with similar issues when making my tool :) |
@RicBent Thank you for your reply! // Embind specific: If enabled, generate Embind's JavaScript invoker functions
// at compile time and include them in the JS output file. When used with
// DYNAMIC_EXECUTION=0 this allows exported bindings to be just as fast as
// DYNAMIC_EXECUTION=1 mode, but without the need for eval(). If there are many
// bindings the JS output size may be larger though.
var EMBIND_AOT = false;
Since unsafe eval() scripts are going deprecated on web for security issues, I thought it would be nice for kiwi-nlp to be eval-free on default. I made a repo to check the difference between 2 versions of kiwi-wasm.js : You can also git clone and run the demo to see it running smoothly. |
Nice research! I totally agree that getting rid of This also seems to work flawlessly in the few tests I did on my end. Seems like worth opening a PR for from your end then? (also those dist files should go onto the gitignore/be removed oops). Might also be a nice idea to have automated JS side tests at some point to check not only for correctness but also performance. Especially considering the broad configurability emscripten has. |
This pull requests adds a build target for a JavaScript library, usable in any modern Browser.
For this the emscripten toolchain is utilized.
Solves #169
You can check out a running demo here: https://ricbent.github.io/Kiwi/demo/
Things that can be improved:
More of the base API functionality (multiple results, custom builder arguments, etc)Addition of GitHub actions to build this new targetCreating a proper npm package (package.json, helpers like the WebWorker wrapper from the demo, TypeScript types, etc), deployed via GitHub actionsAdd DocumentationBecause I haven't set up GitHub actions for this target yet, you need to manually build. This requires the emscripten toolchain to be installed:
mkdir build cd build emcmake cmake -DCMAKE_BUILD_TYPE=Release -DKIWI_USE_CPUINFO=OFF -DKIWI_BUILD_TEST=OFF -DKIWI_USE_MIMALLOC=OFF -DKIWI_BUILD_CLI=OFF -DKIWI_BUILD_EVALUATOR=OFF -DKIWI_BUILD_MODEL_BUILDER=OFF ../ make
This will generate
kiwi-wasm.js
andkiwi-wasm.wasm
. The former can be directly used in any modern browser.Sorry if I didn't follow some contribution guidelines correctly as I don't speak much Korean (yet).
Things changed since the creation of the PR: