No path resolution for ambient modules#455
Conversation
src/LuaTransformer.ts
Outdated
| let requireCall: tstl.CallExpression; | ||
| if (type && this.isAmbientModuleDeclaration(type)) { | ||
| requireCall = tstl.createCallExpression( | ||
| tstl.createIdentifier("require"), |
There was a problem hiding this comment.
Might be nicer to move this logic to createModuleRequire by adding an optional resolveModule: bool = true parameter.
src/LuaTransformer.ts
Outdated
| } | ||
|
|
||
| public isAmbientModuleDeclaration(type: ts.Type): boolean { | ||
| return type.symbol && type.symbol.valueDeclaration.kind === ts.SyntaxKind.ModuleDeclaration; |
There was a problem hiding this comment.
Typically, these types of functions are put in TSHelper
|
I don't think it's good to give |
src/TSHelper.ts
Outdated
| } | ||
|
|
||
| public static isAmbientModuleDeclaration(type: ts.Type): boolean { | ||
| return type.symbol && type.symbol.valueDeclaration.kind === ts.SyntaxKind.ModuleDeclaration; |
There was a problem hiding this comment.
After some discussion in the discord we came to the conclusion that it would be a better solution to add a compiler directive @noResolution on the module, to minimize the risk of this breaking something.
test/unit/require.spec.ts
Outdated
| const regex = /require\("(.*?)"\)/; | ||
| const lua = util.transpileString(` | ||
| import * as fake from "fake"; | ||
| declare module "fake" {} |
There was a problem hiding this comment.
Does this test do anything? You didn't decorate your module with the decorator. (Also the name is misleading)
There was a problem hiding this comment.
Nope it did not. It doesn't look like you can declare a module and import it in the same file. I've made a change to the StringCompilerProgram to allow multiple files to be considered during compilation so I can put through two files to try out this decorator.
src/Compiler.ts
Outdated
| input: string, | ||
| options: CompilerOptions = defaultCompilerOptions, | ||
| filePath = "file.ts", | ||
| extraFiles?: { [filename: string]: string }): ts.Program { |
There was a problem hiding this comment.
I kinda dislike this last parameter, I think it would be nicer if you change the type of input to be string | {[fileName: string]: string}, and then use a typeof check in the function what you are actually doing. (Maybe you could even remove the last parameter - depending on how much that breaks - since you can then do the same with the first parameter).
4 participants
TypeScript allows the following.
mod.d.ts
src/main.ts
tstlmakes the require pathsrc.modbecause it tried to resolve the path.tscdoesn't resolve these, it stays asmod.This PR it to make sure that paths to ambient modules are not resolved.