Skip to content

Commit cac1656

Browse files
PerryvwPerryvw
authored
Module resolution fixes (#1060)
* Deduplicate resolved files * Add resolution context with cache * Fix problem resolving lua siblings * Added forgotten path.normalize * add ignored test node_modules .d.ts * Fix lua files in sources no longer correctly resolving * fix prettier * Fix lua source file resolution this time for real
1 parent 69a44a6 commit cac1656

File tree

11 files changed

+230
-48
lines changed

11 files changed

+230
-48
lines changed

‎src/transpilation/resolve.ts‎

Lines changed: 141 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -21,29 +21,110 @@ interface ResolutionResult {
2121
diagnostics: ts.Diagnostic[];
2222
}
2323

24+
class ResolutionContext {
25+
private resultsCache = new Map<string, ResolutionResult>();
26+
27+
constructor(
28+
public readonly program: ts.Program,
29+
public readonly options: CompilerOptions,
30+
private readonly emitHost: EmitHost
31+
) {}
32+
33+
public resolve(file: ProcessedFile, required: string): ResolutionResult {
34+
const resolvedDependency = resolveDependency(file, required, this.program, this.emitHost);
35+
if (resolvedDependency) {
36+
if (this.options.tstlVerbose) {
37+
console.log(`Resolved ${required} to ${normalizeSlashes(resolvedDependency)}`);
38+
}
39+
40+
// Figure out resolved require path and dependency output path
41+
const resolvedRequire = getEmitPathRelativeToOutDir(resolvedDependency, this.program);
42+
43+
if (shouldRewriteRequires(resolvedDependency, this.program)) {
44+
replaceRequireInCode(file, required, resolvedRequire);
45+
replaceRequireInSourceMap(file, required, resolvedRequire);
46+
}
47+
48+
// Check cache to prevent resolving nested dependencies double to break dependency loops
49+
if (this.resultsCache.has(resolvedDependency)) {
50+
if (this.options.tstlVerbose) {
51+
console.log(`Resolution cache hit for ${normalizeSlashes(resolvedDependency)}`);
52+
}
53+
return this.resultsCache.get(resolvedDependency)!;
54+
}
55+
56+
// If dependency is not part of project, add dependency to output and resolve its dependencies recursively
57+
if (shouldIncludeDependency(resolvedDependency, this.program)) {
58+
// If dependency resolved successfully, read its content
59+
const dependencyContent = this.emitHost.readFile(resolvedDependency);
60+
if (dependencyContent === undefined) {
61+
return { resolvedFiles: [], diagnostics: [couldNotReadDependency(resolvedDependency)] };
62+
}
63+
64+
const dependency = {
65+
fileName: resolvedDependency,
66+
code: dependencyContent,
67+
};
68+
const nestedDependencies = resolveFileDependencies(dependency, this);
69+
70+
// Cache result and return
71+
const result = {
72+
resolvedFiles: [dependency, ...nestedDependencies.resolvedFiles],
73+
diagnostics: [...nestedDependencies.diagnostics],
74+
};
75+
this.resultsCache.set(resolvedDependency, result);
76+
return result;
77+
} else {
78+
const result = {
79+
resolvedFiles: [],
80+
diagnostics: [],
81+
};
82+
this.resultsCache.set(resolvedDependency, result);
83+
return result;
84+
}
85+
} else {
86+
const fallbackRequire = fallbackResolve(required, getSourceDir(this.program), path.dirname(file.fileName));
87+
replaceRequireInCode(file, required, fallbackRequire);
88+
replaceRequireInSourceMap(file, required, fallbackRequire);
89+
90+
return {
91+
resolvedFiles: [],
92+
diagnostics: [
93+
couldNotResolveRequire(required, path.relative(getProjectRoot(this.program), file.fileName)),
94+
],
95+
};
96+
}
97+
}
98+
}
99+
24100
export function resolveDependencies(program: ts.Program, files: ProcessedFile[], emitHost: EmitHost): ResolutionResult {
25101
const outFiles: ProcessedFile[] = [...files];
26102
const diagnostics: ts.Diagnostic[] = [];
27103
const options = program.getCompilerOptions() as CompilerOptions;
28104

105+
const resolutionContext = new ResolutionContext(program, options, emitHost);
106+
29107
// Resolve dependencies for all processed files
30108
for (const file of files) {
31109
if (options.tstlVerbose) {
32110
console.log(`Resolving dependencies for ${normalizeSlashes(file.fileName)}`);
33111
}
34112

35-
const resolutionResult = resolveFileDependencies(file, program, emitHost);
113+
const resolutionResult = resolveFileDependencies(file, resolutionContext);
36114
outFiles.push(...resolutionResult.resolvedFiles);
37115
diagnostics.push(...resolutionResult.diagnostics);
38116
}
39117

40-
return { resolvedFiles: outFiles, diagnostics };
118+
return { resolvedFiles: deduplicateResolvedFiles(outFiles), diagnostics };
41119
}
42120

43-
function resolveFileDependencies(file: ProcessedFile, program: ts.Program, emitHost: EmitHost): ResolutionResult {
121+
function deduplicateResolvedFiles(files: ProcessedFile[]): ProcessedFile[] {
122+
return [...new Map(files.map(f => [f.fileName, f])).values()];
123+
}
124+
125+
function resolveFileDependencies(file: ProcessedFile, context: ResolutionContext): ResolutionResult {
44126
const dependencies: ProcessedFile[] = [];
45127
const diagnostics: ts.Diagnostic[] = [];
46-
const options = program.getCompilerOptions() as CompilerOptions;
47128

48129
for (const required of findRequiredPaths(file.code)) {
49130
// Do no resolve lualib
@@ -60,57 +141,22 @@ function resolveFileDependencies(file: ProcessedFile, program: ts.Program, emitH
60141
}
61142

62143
// Try to resolve the import starting from the directory `file` is in
63-
const fileDir = path.dirname(file.fileName);
64-
const resolvedDependency = resolveDependency(fileDir, required, program, emitHost);
65-
if (resolvedDependency) {
66-
if (options.tstlVerbose) {
67-
console.log(`Resolved ${required} to ${normalizeSlashes(resolvedDependency)}`);
68-
}
69-
70-
// Figure out resolved require path and dependency output path
71-
const resolvedRequire = getEmitPathRelativeToOutDir(resolvedDependency, program);
72-
73-
if (shouldRewriteRequires(resolvedDependency, program)) {
74-
replaceRequireInCode(file, required, resolvedRequire);
75-
replaceRequireInSourceMap(file, required, resolvedRequire);
76-
}
77-
78-
// If dependency is not part of project, add dependency to output and resolve its dependencies recursively
79-
if (shouldIncludeDependency(resolvedDependency, program)) {
80-
// If dependency resolved successfully, read its content
81-
const dependencyContent = emitHost.readFile(resolvedDependency);
82-
if (dependencyContent === undefined) {
83-
diagnostics.push(couldNotReadDependency(resolvedDependency));
84-
continue;
85-
}
86-
87-
const dependency = {
88-
fileName: resolvedDependency,
89-
code: dependencyContent,
90-
};
91-
const nestedDependencies = resolveFileDependencies(dependency, program, emitHost);
92-
dependencies.push(dependency, ...nestedDependencies.resolvedFiles);
93-
diagnostics.push(...nestedDependencies.diagnostics);
94-
}
95-
} else {
96-
// Could not resolve dependency, add a diagnostic and make some fallback path
97-
diagnostics.push(couldNotResolveRequire(required, path.relative(getProjectRoot(program), file.fileName)));
98-
99-
const fallbackRequire = fallbackResolve(required, getSourceDir(program), fileDir);
100-
replaceRequireInCode(file, required, fallbackRequire);
101-
replaceRequireInSourceMap(file, required, fallbackRequire);
102-
}
144+
const resolvedDependency = context.resolve(file, required);
145+
dependencies.push(...resolvedDependency.resolvedFiles);
146+
diagnostics.push(...resolvedDependency.diagnostics);
103147
}
104148
return { resolvedFiles: dependencies, diagnostics };
105149
}
106150

107151
function resolveDependency(
108-
fileDirectory: string,
152+
requiringFile: ProcessedFile,
109153
dependency: string,
110154
program: ts.Program,
111155
emitHost: EmitHost
112156
): string | undefined {
113157
const options = program.getCompilerOptions() as CompilerOptions;
158+
159+
const fileDirectory = path.dirname(requiringFile.fileName);
114160
if (options.tstlVerbose) {
115161
console.log(`Resolving "${dependency}" from ${normalizeSlashes(fileDirectory)}`);
116162
}
@@ -132,10 +178,24 @@ function resolveDependency(
132178
}
133179
}
134180

181+
// Check if this is a lua file in the project sources
182+
const possibleLuaProjectFiles = [
183+
resolvedPath + ".lua", // lua file in sources
184+
path.join(resolvedPath, "index.lua"), // lua index file in sources
185+
];
186+
187+
for (const possibleFile of possibleLuaProjectFiles) {
188+
if (emitHost.fileExists(possibleFile)) {
189+
return possibleFile;
190+
}
191+
}
192+
135193
// Check if this is a sibling of a required lua file
136-
const luaFilePath = path.resolve(fileDirectory, dependency + ".lua");
137-
if (emitHost.fileExists(luaFilePath)) {
138-
return luaFilePath;
194+
if (requiringFile.fileName.endsWith(".lua")) {
195+
const luaFilePath = resolveLuaPath(fileDirectory, dependency, emitHost);
196+
if (luaFilePath) {
197+
return luaFilePath;
198+
}
139199
}
140200

141201
// Not a TS file in our project sources, use resolver to check if we can find dependency
@@ -151,6 +211,39 @@ function resolveDependency(
151211
return undefined;
152212
}
153213

214+
function resolveLuaPath(fromPath: string, dependency: string, emitHost: EmitHost) {
215+
const splitDependency = dependency.split(".");
216+
if (splitDependency.length === 1) {
217+
// If dependency has just one part (the file), look for a lua file with that name
218+
const fileDirectory = walkUpFileTreeUntil(fromPath, dir =>
219+
emitHost.fileExists(path.join(dir, dependency) + ".lua")
220+
);
221+
if (fileDirectory) {
222+
return path.join(fileDirectory, dependency) + ".lua";
223+
}
224+
} else {
225+
// If dependency has multiple parts, look for the first directory of the require path, which must be in the lua root
226+
const luaRoot = walkUpFileTreeUntil(fromPath, dir =>
227+
emitHost.directoryExists(path.join(dir, splitDependency[0]))
228+
);
229+
if (luaRoot) {
230+
return path.join(luaRoot, dependency.replace(".", path.sep)) + ".lua";
231+
}
232+
}
233+
}
234+
235+
function walkUpFileTreeUntil(fromDirectory: string, predicate: (dir: string) => boolean) {
236+
const currentDir = path.normalize(fromDirectory).split(path.sep);
237+
while (currentDir.length > 0) {
238+
const dir = currentDir.join(path.sep);
239+
if (predicate(dir)) {
240+
return dir;
241+
}
242+
currentDir.pop();
243+
}
244+
return undefined;
245+
}
246+
154247
function shouldRewriteRequires(resolvedDependency: string, program: ts.Program) {
155248
return !isNodeModulesFile(resolvedDependency) || !isBuildModeLibrary(program);
156249
}

‎src/transpilation/utils.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import * as lua from "../LuaAST";
88
import * as diagnosticFactories from "./diagnostics";
99

1010
export interface EmitHost {
11+
directoryExists(path: string): boolean;
1112
fileExists(path: string): boolean;
1213
getCurrentDirectory(): string;
1314
readFile(path: string): string | undefined;

‎test/transpile/module-resolution.spec.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,3 +275,37 @@ describe("module resolution with tsx", () => {
275275
});
276276
});
277277
});
278+
279+
describe("dependency with complicated inner structure", () => {
280+
const projectPath = path.resolve(__dirname, "module-resolution", "project-with-complicated-dependency");
281+
const tsConfigPath = path.join(projectPath, "tsconfig.json");
282+
const mainFilePath = path.join(projectPath, "main.ts");
283+
284+
const expectedResult = {
285+
otherFileResult: "someFunc from otherfile.lua",
286+
otherFileUtil: "util",
287+
utilResult: "util",
288+
};
289+
290+
// Test fix for https://github.com/TypeScriptToLua/TypeScriptToLua/issues/1055
291+
test("bundle should not contain duplicate files", () => {
292+
const mainFile = path.join(projectPath, "main.ts");
293+
const { transpiledFiles } = util
294+
.testProject(tsConfigPath)
295+
.setMainFileName(mainFilePath)
296+
.setOptions({ luaBundle: "bundle.lua", luaBundleEntry: mainFile })
297+
.expectToEqual(expectedResult)
298+
.getLuaResult();
299+
300+
expect(transpiledFiles).toHaveLength(1);
301+
const lua = transpiledFiles[0].lua!;
302+
// util is used in 2 places, but should only occur once in the bundle
303+
const utilModuleOccurrences = (lua.match(/\["lua_modules\.dependency1\.util"\]/g) ?? []).length;
304+
expect(utilModuleOccurrences).toBe(1);
305+
});
306+
307+
// Test fix for https://github.com/TypeScriptToLua/TypeScriptToLua/issues/1054
308+
test("should be able to resolve dependency files in subdirectories", () => {
309+
util.testProject(tsConfigPath).setMainFileName(mainFilePath).expectToEqual(expectedResult);
310+
});
311+
});

‎test/transpile/module-resolution/project-with-complicated-dependency/main.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import * as dependency1 from "dependency1";
2+
3+
export const otherFileResult = dependency1.otherFileFromDependency1();
4+
export const utilResult = dependency1.callUtil();
5+
export const otherFileUtil = dependency1.otherFileUtil();

‎test/transpile/module-resolution/project-with-complicated-dependency/node_modules/dependency1/index.d.ts‎

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/transpile/module-resolution/project-with-complicated-dependency/node_modules/dependency1/index.lua‎

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/transpile/module-resolution/project-with-complicated-dependency/node_modules/dependency1/otherfile.lua‎

Lines changed: 10 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/transpile/module-resolution/project-with-complicated-dependency/node_modules/dependency1/subdir/othersubdirfile.lua‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/transpile/module-resolution/project-with-complicated-dependency/node_modules/dependency1/subdir/subdirfile.lua‎

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/transpile/module-resolution/project-with-complicated-dependency/node_modules/dependency1/util.lua‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)