openpeeps / denim

Node 💖 Nim = Denim! Build powerful NodeJS / BunJS addons with Nim language via Node API (NAPI)
https://openpeeps.github.io/denim/
MIT License
40 stars 1 forks source link

Code breaks with latest head #6

Closed jfilby closed 1 year ago

jfilby commented 1 year ago

I installed the latest denim@#head to try the high level API, but unfortunately it doesn't work for me, and the low-level API seems to be broken too.

Low-level API has two problems:

  1. The Env.expect call I had no longer works:
Error: type mismatch: got <napi_env, seq[napi_value], string, (string, NapiValueType), (string, NapiValueType)>
but expected one of:
proc expect(env: napi_env; n: napi_value; expectKind: NapiValueType): bool
  first type mismatch at position: 2
  required type for n: napi_value
  but expression 'args' is of type: seq[napi_value]
proc expect(env: napi_env; v: seq[napi_value]; errorName, fnIdent: string): bool
  first type mismatch at position: 4
  required type for fnIdent: string
  but expression '("accountUserId", napi_number)' is of type: (string, NapiValueType)
  1. Node-gyp gives an unhelpful error: Error: unhandled exception: field 'vStr' is not accessible for type 'Parameter' using 'ptype = LongFlag' [FieldDefect]

The high-level API didn't work for me either, also two problems:

  1. The example on the GitHub README.md errors on a warning line:
Error: expression expected, but found 'A simple function from Nim'
  1. Calling args.get("name") also didn't work for me.
georgelemon commented 1 year ago

If you want to use the new High-level API {.export_napi} there is no need to check for types. Consider the following:

init proc(module: Module) =
  proc callNimFn(name: string): string {.export_napi} =
    # `callNimFn` is not a real proc, here we'll use nnkProcDef to retrieve
    # nnkFormalParams for creating the auto type checker. 

    # types supported (Nim > NAPI):
    # string > napi_string
    # int > napi_number
    # object > napi_object
    # array > napi_object (cuz it's an object that) here we use `napi_is_array`
    # undefined > napi_undefined
    # nil > napi_null
    # symbol > napi_symbol
    # func > napi_function
    # external > napi_external
    # varargs[string] > variable arguments (unstable)

    # optional args should be called like (not implemented, yet):
    # age: int = 21
    # or
    # age = 21 

    # the return type is not required. but is going to be used
    # when generating the docblock comment for `@return {object}`

    # args.get("name") is not implemented, yet
    # so you will have to keep using `args` sequence.
    return args[0].getStr

And yes, I will make expect proc work with the old module.registerFn()

georgelemon commented 1 year ago

Reinstalling denim should fix a little bug around getNimNapiType

georgelemon commented 1 year ago

Note that you can npm install cmake-js -g. Is faster than node-gyp

denim build src/myapp.nim --cmake --yes

Node-gyp gives an unhelpful error: Error: unhandled exception: field 'vStr' is not accessible for type 'Parameter' using 'ptype = LongFlag' [FieldDefect]

There is a little bug in kapsis cli. I will try fix this soon

georgelemon commented 1 year ago

Will keep my README simple, with a minimal example.

Anything else will be in tests.

jfilby commented 1 year ago

Thanks. I'm trying cmake-js, it does seem faster on repeated calls. I'll wait for the fix to kapsis.

jfilby commented 1 year ago

When you make the fix to kapsis can you please make a release, and set denim's dependency to that new version of kapsis? Also please then release a new version of denim. It's just easier to develop against specific versions than head.

georgelemon commented 1 year ago

Not quite ready for a release, but you can reinstall from head. Should work for now.

Also, check /tests for fully working examples.

georgelemon commented 1 year ago

Just released 0.1.5. Updated docs

jfilby commented 1 year ago

Works great, thanks!