Default null to nil when injectObjects is not set

[gudzpoz]

The PR #101 handles null values only when injectObjects is set to true. When injectObjects is undefined or false, null values are not handled at all, somehow triggering a TypeError in PromiseTypeExtension or otherwise a The type 'object' is not supported by Lua error.

const { LuaFactory } = require('wasmoon')
const L = await factory.createEngine({ injectObjects: false })
L.global.pushValue(null) // TypeError here

Uncaught TypeError: Cannot read properties of null (reading 'then')
    at PromiseTypeExtension.pushValue (.../node_modules/.pnpm/[email protected]/node_modules/wasmoon/dist/index.js:916:102)
    at .../node_modules/.pnpm/[email protected]/node_modules/wasmoon/dist/index.js:244:82
    at Array.find (<anonymous>)
    at Global.pushValue (.../node_modules/.pnpm/[email protected]/node_modules/wasmoon/dist/index.js:244:46)

This PR adds a simple DefaultNullTypeExtension for cases when injectObjects is false.

fixes #104

[tims-bsquare]

This is one of those cases where I think your changes should be in thread.ts.

It looks like currently returning nil from Lua will map to JS null but pushing null doesn't correctly work which sounds like a bug there to me.

However, I'm not sure which should be the default, mapping to null or to undefined. Personally I'd say undefined because it's closer to what nil means in Lua but it is currently null on a return nil.

@ceifa I think supporting null in thread.ts:pushValue would be handy along with being sure you'd like to return null from thread.ts:getValue

[tims-bsquare]

This should've remained in if you were doing a type extension imo. I'd have said you should inject the default null type extension always with a lower priority and then allow this to override it

[gudzpoz]

Thanks for the suggestion! It occurs to me that default null values can be handled in type-extensions/table.ts instead (as is done in the latest commit). Or do you prefer it to be handled in a standalone handler?

By the way, I can't think of a good way to move the changes into thread.ts. Do you mean something like this?

diff --git a/src/thread.ts b/src/thread.ts
index 12e26f4..53387e0 100755
--- a/src/thread.ts
+++ b/src/thread.ts
@@ -216,6 +216,12 @@ export default class Thread {
             case 'boolean':
                 this.lua.lua_pushboolean(this.address, target ? 1 : 0)
                 break
+            case 'object':
+                if (target === null && !((this.parent ?? this) as unknown as Global).injectObject) {
+                    this.lua.lua_pushnil(this.address)
+                    break
+                }
+            // Otherwise, fall through
             default:
                 if (!this.typeExtensions.find((wrapper) => wrapper.extension.pushValue(this, decoratedValue, userdata))) {
                     throw new Error(`The type '${typeof target}' is not supported by Lua`)

[tims-bsquare]

Thanks for the suggestion! It occurs to me that default null values can be handled in type-extensions/table.ts instead (as is done in the latest commit). Or do you prefer it to be handled in a standalone handler?

By the way, I can't think of a good way to move the changes into thread.ts. Do you mean something like this?

diff --git a/src/thread.ts b/src/thread.ts
index 12e26f4..53387e0 100755
--- a/src/thread.ts
+++ b/src/thread.ts
@@ -216,6 +216,12 @@ export default class Thread {
             case 'boolean':
                 this.lua.lua_pushboolean(this.address, target ? 1 : 0)
                 break
+            case 'object':
+                if (target === null && !((this.parent ?? this) as unknown as Global).injectObject) {
+                    this.lua.lua_pushnil(this.address)
+                    break
+                }
+            // Otherwise, fall through
             default:
                 if (!this.typeExtensions.find((wrapper) => wrapper.extension.pushValue(this, decoratedValue, userdata))) {
                     throw new Error(`The type '${typeof target}' is not supported by Lua`)

Personally I think it should be handled in thread.ts with the other primitives, definitely not in the table extension though, it may be an object but not one with keys. The downside of that being you'd still need to allow extensions to overwrite null.

In thread.ts I think what you'd have to do is in the default case when no extensions are found then if the value is null then push nil and the same in reverse for the get.

To be clear though it may be better as a type extension separate from the current null and table types. I only suggest thread.ts because there's already some null/undefined handling in there.

[tims-bsquare]

I'm happy with this if you are @ceifa

Thanks for the iterations @gudzpoz

[ceifa]

LGTM