Skip to content

Implemented typeof, instanceof and overloads - #128

Merged
Perryvw merged 7 commits into
masterfrom
feature/overloads
Jul 1, 2018
Merged

Implemented typeof, instanceof and overloads#128
Perryvw merged 7 commits into
masterfrom
feature/overloads

Conversation

@Perryvw

@Perryvw Perryvw commented Jun 30, 2018

Copy link
Copy Markdown
Member

Implemented support for typeof, instanceof and the ability to deal with overloads for #122. Also found a random unsupported string type that was added.

@Perryvw
Perryvw requested a review from lolleko June 30, 2018 15:19
@dmarcuse

Copy link
Copy Markdown
Contributor

This typeof implementation could cause some issues with type narrowing: IIRC, TS expects typeof to return "string" | "number" | "array" | "function" | "boolean" | "object", while lua's type will return "table" instead of "array" or "object".

@Perryvw

Perryvw commented Jun 30, 2018

Copy link
Copy Markdown
Member Author

I don't think array is a thing, I can't find any references to MDN, also typeof([]) gives me "object" in Chrome. I was discussing with @lolleko wether we should be returning "table" or "object", I think table should be fine as long as we document it properly.

@dmarcuse

dmarcuse commented Jul 1, 2018

Copy link
Copy Markdown
Contributor

I might have misremembered that with "array" then. My concern was mostly that if you have const something: object | number, if (typeof something === "table") won't successfully perform type narrowing. In fact, in recent versions of TS, I think it even produces a compiler error if you try to compare an unknown constant to a typeof result.

@Perryvw

Perryvw commented Jul 1, 2018

Copy link
Copy Markdown
Member Author

Ah yes that is a good point, I will change that, we will probably translate to type(...) == "table" and "object" or type(...)

@Perryvw
Perryvw merged commit 714954a into master Jul 1, 2018
@Perryvw
Perryvw deleted the feature/overloads branch July 1, 2018 08:46
@Perryvw Perryvw mentioned this pull request Jul 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants