Skip to content

Implemented typeof, instanceof and overloads#128

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

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

Conversation

@Perryvw
Copy link
Copy Markdown
Member

@Perryvw Perryvw commented Jun 30, 2018

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
Copy link
Copy Markdown
Member Author

Perryvw commented Jun 30, 2018

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
Copy link
Copy Markdown
Contributor

dmarcuse commented Jul 1, 2018

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
Copy link
Copy Markdown
Member Author

Perryvw commented Jul 1, 2018

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