Skip to content
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.

Sign up for GitHub

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

Building TypeScript type definitions, along with some JSDoc #210

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Building TypeScript type definitions, along with some JSDoc #210

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
b627429
Generate type definitions with tsc
gudzpoz Apr 27, 2023
40bfd91
src/defs.js: add jsdoc with typing info
gudzpoz Apr 27, 2023
b400812
src/luaconf.js: add typing info
gudzpoz Apr 27, 2023
d7bc2fe
src/lapi.js: add type info
gudzpoz Apr 27, 2023
71d42ec
src/lualib.js: add type info
gudzpoz Apr 27, 2023
16a4a59
src/lauxlib.js: add type info
gudzpoz Apr 27, 2023
64aaa89
Fix incorrect signatures with tsc checkJs
gudzpoz Apr 27, 2023
477f93f
Workaround possible tsc caveat when re-exporting
gudzpoz Apr 27, 2023
2c73341
Bug fixes and typing tests with checkJs
gudzpoz Apr 27, 2023
7523f53
Reverse several unnecessary changes
gudzpoz Apr 27, 2023
81472fb
Test type info with test files
gudzpoz Apr 28, 2023
8eec9d5
Add a os.rename/remove test
gudzpoz Apr 28, 2023
4e3dc6a
Use LuaString as a type alias for Uint8Array
gudzpoz Apr 28, 2023
a6476ff
Ask tsc to merge all d.ts files into one
gudzpoz Apr 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,16 @@
"version": "0.1.4",
"description": "A Lua VM written in JS ES6 targeting the browser",
"main": "src/fengari.js",
"types": "dist/fengari.d.ts",
"directories": {
"lib": "src",
"test": "test"
},
"scripts": {
"emit": "tsc",
"lint": "eslint src/ test/",
"prepublishOnly": "git diff-index --quiet --cached HEAD -- && npm run lint && npm run test",
"test": "jest"
"test": "npm run emit && jest"
},
"repository": {
"type": "git",
Expand All @@ -30,8 +32,10 @@
},
"homepage": "https://github.com/fengari-lua/fengari#readme",
"devDependencies": {
"@types/node": "^18.16.1",
"eslint": "^5.15.1",
"jest": "^24.5.0"
"jest": "^24.5.0",
"typescript": "^5.0.4"
},
"dependencies": {
"readline-sync": "^1.4.9",
Expand Down
80 changes: 75 additions & 5 deletions src/defs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@
* Fengari specific string conversion functions
*/

/**
* Converts a JavaScript string into a Uint8Array (used a Lua string).
gudzpoz marked this conversation as resolved.
Show resolved Hide resolved
*
* @type {function(ArrayLike<number>):Uint8Array}
*/
let luastring_from;
if (typeof Uint8Array.from === "function") {
luastring_from = Uint8Array.from.bind(Uint8Array);
Expand All @@ -17,20 +22,30 @@ if (typeof Uint8Array.from === "function") {
};
}

/**
* Returns the index of the first occurrence of the character in the Lua string.
*
* @type {function(Uint8Array|any[], number, number=):number}
*/
let luastring_indexOf;
if (typeof (new Uint8Array().indexOf) === "function") {
luastring_indexOf = function(s, v, i) {
return s.indexOf(v, i);
};
} else {
/* Browsers that don't support Uint8Array.indexOf seem to allow using Array.indexOf on Uint8Array objects e.g. IE11 */
let array_indexOf = [].indexOf;
let array_indexOf = [0].indexOf;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. I'm sorta surprised this works

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually because TypeScript treats [] as Array<undefined> and forbids using it as Array<number>. Probably using Array.prototype.indexOf is more understandable.

if (array_indexOf.call(new Uint8Array(1), 0) !== 0) throw Error("missing .indexOf");
luastring_indexOf = function(s, v, i) {
return array_indexOf.call(s, v, i);
};
}

/**
* Constructs a Lua string from characters.
*
* @type {function(...number):Uint8Array}
*/
let luastring_of;
if (typeof Uint8Array.of === "function") {
luastring_of = Uint8Array.of.bind(Uint8Array);
Expand All @@ -40,11 +55,22 @@ if (typeof Uint8Array.of === "function") {
};
}

/**
* Returns `true` if the object can be used as a Lua string.
*
* @param {any} s the object
*/
const is_luastring = function(s) {
return s instanceof Uint8Array;
};

/* test two lua strings for equality */
/**
* Tests two Lua strings for equality.
*
* @param {Uint8Array} a str 1
* @param {Uint8Array} b str 2
* @returns {boolean}
*/
const luastring_eq = function(a, b) {
if (a !== b) {
let len = a.length;
Expand All @@ -57,6 +83,16 @@ const luastring_eq = function(a, b) {
};

const unicode_error_message = "cannot convert invalid utf8 to javascript string";

/**
* Converts a Lua string (in UTF-8) to a normal JavaScript string.
*
* @param {Uint8Array} value the Lua string
* @param {number} [from] the staring index
* @param {number} [to] the ending index
* @param {boolean} [replacement_char] whether to replace invalid utf8 chars
* @returns {string}
*/
const to_jsstring = function(value, from, to, replacement_char) {
if (!is_luastring(value)) throw new TypeError("to_jsstring expects a Uint8Array");

Expand Down Expand Up @@ -159,7 +195,12 @@ const uri_allowed = (";,/?:@&=+$abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUV
return uri_allowed;
}, {});

/* utility function to convert a lua string to a js string with uri escaping */
/**
* Utility function to convert a Lua string to a JavaScript string with URI escaping.
*
* @param {Uint8Array} a the string
* @returns {string}
*/
const to_uristring = function(a) {
if (!is_luastring(a)) throw new TypeError("to_uristring expects a Uint8Array");
let s = "";
Expand All @@ -176,6 +217,11 @@ const to_uristring = function(a) {

const to_luastring_cache = {};

/**
* @param {string} str
* @param {boolean} [cache]
* @returns {Uint8Array}
*/
const to_luastring = function(str, cache) {
if (typeof str !== "string") throw new TypeError("to_luastring expects a javascript string");

Expand All @@ -185,6 +231,7 @@ const to_luastring = function(str, cache) {
}

let len = str.length;
/** @type {Array<number> | Uint8Array} */
let outU8Array = Array(len); /* array is at *least* going to be length of string */
let outIdx = 0;
for (let i = 0; i < len; ++i) {
Expand Down Expand Up @@ -224,6 +271,15 @@ const to_luastring = function(str, cache) {
return outU8Array;
};

/**
* Returns a Lua string.
*
* If `str` is already a Lua string, it returns it as is.
* Otherwise, it tries to convert it.
*
* @param {string|Uint8Array} str
* @returns {Uint8Array}
*/
const from_userstring = function(str) {
if (!is_luastring(str)) {
if (typeof str === "string") {
Expand All @@ -232,6 +288,7 @@ const from_userstring = function(str) {
throw new TypeError("expects an array of bytes or javascript string");
}
}
// @ts-ignore
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed? / is there some other decorator we could use?

Copy link
Author

@gudzpoz gudzpoz Apr 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, TypeScript is called AnyScript for a reason - it fails to deduce that str has to be Uint8Array. An alternative is to make things more explicit:

 const from_userstring = function(str) {
-    if (!is_luastring(str)) {
+    if (!(str instanceof Uint8Array)) {

(One may also use // @ts-expect-error, but neither offers a way to specify the suppressed error type.)

return str;
};

Expand Down Expand Up @@ -269,7 +326,6 @@ module.exports.LUA_RELEASE = LUA_RELEASE;
module.exports.LUA_COPYRIGHT = LUA_COPYRIGHT;
module.exports.LUA_AUTHORS = LUA_AUTHORS;


const thread_status = {
LUA_OK: 0,
LUA_YIELD: 1,
Expand All @@ -291,7 +347,15 @@ const constant_types = {
LUA_TFUNCTION: 6,
LUA_TUSERDATA: 7,
LUA_TTHREAD: 8,
LUA_NUMTAGS: 9
LUA_NUMTAGS: 9,

LUA_TSHRSTR: 0,
LUA_TLNGSTR: 0,
LUA_TNUMFLT: 0,
LUA_TNUMINT: 0,
LUA_TLCL: 0,
LUA_TLCF: 0,
LUA_TCCL: 0,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this makes the code harder to understand

Copy link
Author

@gudzpoz gudzpoz Apr 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TypeScript does not track type info of the fields assigned only after object initialization. What about:

const LUA_OK        = 0;
const LUA_YIELD     = 1;
const LUA_ERRRUN    = 2;
const LUA_ERRSYNTAX = 3;
const LUA_ERRMEM    = 4;
const LUA_ERRGCMM   = 5;
const LUA_ERRERR    = 6;
const LUA_TSHRSTR   = LUA_TSTRING | (0 << 4);  /* short strings */
const LUA_TLNGSTR   = LUA_TSTRING | (1 << 4);  /* long strings */
const LUA_TNUMFLT   = LUA_TNUMBER | (0 << 4);  /* float numbers */
const LUA_TNUMINT   = LUA_TNUMBER | (1 << 4);  /* integer numbers */
const LUA_TLCL      = LUA_TFUNCTION | (0 << 4);  /* Lua closure */
const LUA_TLCF      = LUA_TFUNCTION | (1 << 4);  /* light C function */
const LUA_TCCL      = LUA_TFUNCTION | (2 << 4);  /* C closure */

const thread_status = {
    LUA_OK,
    LUA_YIELD,
    LUA_ERRRUN,
    LUA_ERRSYNTAX,
    LUA_ERRMEM,
    LUA_ERRGCMM,
    LUA_ERRERR,
    LUA_TSHRSTR,
    LUA_TLNGSTR,
    LUA_TNUMFLT,
    LUA_TNUMINT,
    LUA_TLCL,
    LUA_TLCF,
    LUA_TCCL,
};

};

constant_types.LUA_TSHRSTR = constant_types.LUA_TSTRING | (0 << 4); /* short strings */
Expand Down Expand Up @@ -332,6 +396,12 @@ const LUA_MINSTACK = 20;
const { LUAI_MAXSTACK } = require('./luaconf.js');
const LUA_REGISTRYINDEX = -LUAI_MAXSTACK - 1000;

/**
* Returns a pseudo-index for the i-th upvalue.
*
* @param {number} i
* @returns {number}
*/
const lua_upvalueindex = function(i) {
return LUA_REGISTRYINDEX - i;
};
Expand Down
43 changes: 21 additions & 22 deletions src/fengari.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,28 +10,27 @@ Copyright © 1994–2017 Lua.org, PUC-Rio.

const core = require("./fengaricore.js");

module.exports.FENGARI_AUTHORS = core.FENGARI_AUTHORS;
module.exports.FENGARI_COPYRIGHT = core.FENGARI_COPYRIGHT;
module.exports.FENGARI_RELEASE = core.FENGARI_RELEASE;
module.exports.FENGARI_VERSION = core.FENGARI_VERSION;
module.exports.FENGARI_VERSION_MAJOR = core.FENGARI_VERSION_MAJOR;
module.exports.FENGARI_VERSION_MINOR = core.FENGARI_VERSION_MINOR;
module.exports.FENGARI_VERSION_NUM = core.FENGARI_VERSION_NUM;
module.exports.FENGARI_VERSION_RELEASE = core.FENGARI_VERSION_RELEASE;
const exported = {
FENGARI_AUTHORS: core.FENGARI_AUTHORS,
FENGARI_COPYRIGHT: core.FENGARI_COPYRIGHT,
FENGARI_RELEASE: core.FENGARI_RELEASE,
FENGARI_VERSION: core.FENGARI_VERSION,
FENGARI_VERSION_MAJOR: core.FENGARI_VERSION_MAJOR,
FENGARI_VERSION_MINOR: core.FENGARI_VERSION_MINOR,
FENGARI_VERSION_NUM: core.FENGARI_VERSION_NUM,
FENGARI_VERSION_RELEASE: core.FENGARI_VERSION_RELEASE,

module.exports.luastring_eq = core.luastring_eq;
module.exports.luastring_indexOf = core.luastring_indexOf;
module.exports.luastring_of = core.luastring_of;
module.exports.to_jsstring = core.to_jsstring;
module.exports.to_luastring = core.to_luastring;
module.exports.to_uristring = core.to_uristring;
luastring_eq: core.luastring_eq,
luastring_indexOf: core.luastring_indexOf,
luastring_of: core.luastring_of,
to_jsstring: core.to_jsstring,
to_luastring: core.to_luastring,
to_uristring: core.to_uristring,

const luaconf = require('./luaconf.js');
const lua = require('./lua.js');
const lauxlib = require('./lauxlib.js');
const lualib = require('./lualib.js');
luaconf: require('./luaconf.js'),
lua: require('./lua.js'),
lauxlib: require('./lauxlib.js'),
lualib: require('./lualib.js'),
};

module.exports.luaconf = luaconf;
module.exports.lua = lua;
module.exports.lauxlib = lauxlib;
module.exports.lualib = lualib;
module.exports = exported;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recall that this caused problems in some ancient node version; or was it an issue with particular minifiers?

Copy link
Author

@gudzpoz gudzpoz Apr 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to cause problems when there is a cyclic dependency: https://stackoverflow.com/questions/63371870/object-assignmodule-exports-vs-module-exports
Unfortunately, loadlib.js depending on fengari.js creates exactly a dependency cycle. Maybe we can defer requiring fengari.js (which is only used inside a library loading function)?

(I just tried Object.assign and it turns out TypeScript empties all types definitions, leaving only export {};.)

Loading