-
-
Notifications
You must be signed in to change notification settings - Fork 65
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
Typescript typings: First cut #157
Conversation
Reformat, add some missing API calls, move tslint comments, add Type and arith ip constants.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks good but I don't feel confident: I don't know much typescript.
I'd like to get a second opinion from a regular typescript user. Trying to find one....
const LUA_OK: number; | ||
const LUA_ERRRUN: number; | ||
|
||
type lua_Alloc = (...a: any) => any; // TODO: define this correctly. Is alloc actually used? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good question...
export type lua_Type = number; | ||
export type lua_ArithOp = number; | ||
|
||
const LUA_OK: number; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can type these down to actual literals provided the definitions in defs.js
are actually constants, which means LUA_OK
has the type 0
, LUA_ERRRUN
has the type 2
, etc.
This will be true for constant string values as well, as well as makeshift unions of them, which will effectively rule out the rest of the value space of strings very effectively.
I'd appreciate that. I'm far from a Typescript guru, so peer comments are welcome. |
export const FENGARI_VERSION_RELEASE: string; | ||
export const FENGARI_VERSION_MINOR: string; | ||
|
||
export type lua_String = Uint8Array; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that is_luastring
wasn't typed (though it seems exported?).
A helpful tip about these kind of predicate functions is that they can inform the type system:
const is_luastring = (value: unknown): value is lua_String => {
// If the function returns true the type system knows that this is indeed a `lua_String`
// otherwise not
}
} | ||
|
||
export type lua_Type = number; | ||
export type lua_ArithOp = number; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you type the OP
variables down to literals you can then make lua_ArithOp
a union:
export type lua_ArithOp = LUA_OPADD | LUA_OPSUB | ... | LUA_OPBNOT;
@roddypratt did you want to keep working on this? Is it in a mergable state (the TODOs suggest not; but maybe gradual typing is a thing?) |
I am considering picking this up. One of the big things is that things like |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
Closing due to staleness |
My first github PR, so please be gentle.
This is to address issue #135 by adding a typescript .d.ts file containing typings. At this stage they're incomplete (no enums, maybe missing some functions) and there's likely some confusion about where JS strings vs Lua strings are required.
I've also got a Typescript version of
lapi.test.js
which uses the typings and passes. Not sure if that should be included in this PR?I've got similar PRs waiting in the wings for fengari-interop and fengari-web.