passing nil instead of _G as context for global functions when in ES …#271
Conversation
src/Transpiler.ts
Outdated
| this.sourceFile = sourceFile; | ||
| this.isModule = tsHelper.isFileModule(sourceFile); | ||
| this.isStrict = options.alwaysStrict || options.strict | ||
| || (options.target && options.target > ts.ScriptTarget.ES5); |
There was a problem hiding this comment.
Is ES5+ always strict? Or (if it's default true) did you forget to check if it is not explicitly set to false.
There was a problem hiding this comment.
Ok, I just looked this up in the ecma specs. In ES5, strict mode has to be explicitly enabled (with "use strict" which is what alwaysStrict produces). In ES6 (aka ES2015) and beyond, modules are always strict (I don't believe it can be disabled), but non-modules still require the explicit enabling. I'll update to match that logic.
src/Transpiler.ts
Outdated
| && !ts.isElementAccessExpression(node.expression) | ||
| && !tsHelper.getCustomDecorators(type, this.checker).has(DecoratorKind.NoContext)) { | ||
| params = this.transpileArguments(node.arguments, ts.createIdentifier("_G")); | ||
| params = this.transpileArguments(node.arguments, ts.createIdentifier(this.isStrict ? "nil" : "_G")); |
There was a problem hiding this comment.
Maybe instead of creating an identifier you could just create a null or undefined keyword? I don't know how big that change would be
There was a problem hiding this comment.
The context paramater of transpileArguments is optional and skips adding the initial parameter if omitted (or undefined). I could re-purpose null to explicitly convert to nil but having separate logic for null and undefined always feels icky to me.
I could change transpileArguments to take either an expression or a string as the context parameter, and just pass the "nil" instead of a temp identifier.
There was a problem hiding this comment.
I don't mean actual null, I meant something like ts.CreateNullKeyword() or something similar.
There was a problem hiding this comment.
actually nvm that won't work here since it is an abstract argument and not a concrete instantiation of one.
There was a problem hiding this comment.
Ah - sorry - misunderstood. Yes, that seems like a good idea.
2 participants
As discussed here: #250 (comment)