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.

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

fix: add check to see in ownerSymbol parent is the global symbol #1472

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

starsprung
Copy link

@starsprung starsprung commented Aug 5, 2023

Add check to see in ownerSymbol.parent is the global symbol when determining when to transform global builtin calls.

I'm not sure if this is the appropriate fix, but this change allows calls to console.log to compile correctly when using the types from @types/node and not using the dom lib. They already work correctly with the type declarations in lib.dom.d.ts, due to a difference in structure: Node's typings wrap console inside global { }, which means it has a parent, whereas console from lib.dom.d.ts doesn't.

E.g.
Input

console.log('test');

tstl output without fix

--[[ Generated with https://github.com/TypeScriptToLua/TypeScriptToLua ]]
console:log("hello world")

tstl output with fix

--[[ Generated with https://github.com/TypeScriptToLua/TypeScriptToLua ]]
print("hello world")

when determining when to transform global builtin calls.
@@ -81,7 +81,8 @@ function tryTransformBuiltinGlobalMethodCall(
) {
const ownerType = context.checker.getTypeAtLocation(calledMethod.expression);
const ownerSymbol = tryGetStandardLibrarySymbolOfType(context, ownerType);
if (!ownerSymbol || ownerSymbol.parent) return;
Copy link
Member

Choose a reason for hiding this comment

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

Wondering if instead we shouldn't just be removing this ownerSymbol.parent, not really sure if it adds anything

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.

None yet

2 participants