Skip to content

Commit 53e031b

Browse files
andrewbranchandrewbranch
authored
Fix internal resolution error tracing (#44)
* Only issue UnexpectedModuleSyntax in node16 * Fix trace loading * Dedupe files for ResolutionBasedFileProblems * Rename InternalResolutionError * Add changesets * Update new snapshot
1 parent 7cbd310 commit 53e031b

15 files changed

+2726
-1129
lines changed

‎.changeset/chilly-candles-approve.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@arethetypeswrong/core": patch
3+
---
4+
5+
Fixed an issue where InternalResolutionError and UnexpectedModuleSyntax could be duplicated

‎.changeset/curvy-masks-learn.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@arethetypeswrong/core": patch
3+
---
4+
5+
Fixed an issue where InternalResolutionError was missing traces

‎.changeset/twelve-feet-leave.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@arethetypeswrong/core": minor
3+
---
4+
5+
Inlined `InternalResolutionErrorDetails` into `InternalResolutionError` and renamed the interface to `InternalResolutionErrorProblem` to match other problem interfaces

‎package-lock.json

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

‎package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
"@changesets/cli": "^2.26.1",
2222
"@types/node": "^18.11.18",
2323
"prettier": "^2.8.3",
24-
"typescript": "^5.0.0-dev.20230207"
24+
"typescript": "^5.1.3"
2525
},
2626
"volta": {
2727
"node": "19.8.1"

‎packages/cli/test/snapshots/@[email protected]

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# @[email protected]
2+
3+
```
4+
$ attw @[email protected]
5+
6+
7+
⚠️ A require call resolved to an ESM JavaScript file, which is an error in Node and some bundlers. CommonJS consumers will need to use a dynamic import. https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/CJSResolvesToESM.md
8+
9+
💀 Import failed to resolve to type declarations or JavaScript files. https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/NoResolution.md
10+
11+
🥴 Import found in a type declaration file failed to resolve. Either this indicates that runtime resolution errors will occur, or (more likely) the types misrepresent the contents of the JavaScript files. https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/InternalResolutionError.md
12+
13+
14+
┌────────────────────┬───────────────────────────────────┬───────────────────────────────────┬───────────────────────────────────┐
15+
│ │ "@ice/app" │ "@ice/app/types" │ "@ice/app/analyze" │
16+
├────────────────────┼───────────────────────────────────┼───────────────────────────────────┼───────────────────────────────────┤
17+
│ node10 │ 🟢 │ 🟢 │ 💀 Resolution failed │
18+
├────────────────────┼───────────────────────────────────┼───────────────────────────────────┼───────────────────────────────────┤
19+
│ node16 (from CJS) │ ⚠️ ESM (dynamic import only) │ ⚠️ ESM (dynamic import only) │ ⚠️ ESM (dynamic import only) │
20+
│ │ 🥴 Internal resolution error │ 🥴 Internal resolution error │ │
21+
├────────────────────┼───────────────────────────────────┼───────────────────────────────────┼───────────────────────────────────┤
22+
│ node16 (from ESM) │ 🥴 Internal resolution error │ 🥴 Internal resolution error │ 🟢 (ESM) │
23+
├────────────────────┼───────────────────────────────────┼───────────────────────────────────┼───────────────────────────────────┤
24+
│ bundler │ 🟢 │ 🟢 │ 🟢 │
25+
└────────────────────┴───────────────────────────────────┴───────────────────────────────────┴───────────────────────────────────┘
26+
27+
28+
```
29+
30+
Exit code: 1

‎packages/core/package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,12 @@
5252
"@andrewbranch/untar.js": "^1.0.0",
5353
"fetch-ponyfill": "^7.1.0",
5454
"fflate": "^0.7.4",
55-
"typescript": "^5.0.0-dev.20230207",
55+
"typescript": "^5.1.3",
5656
"validate-npm-package-name": "^5.0.0"
5757
},
5858
"devDependencies": {
5959
"@types/node": "^18.15.11",
60-
"@types/ts-expose-internals": "npm:ts-expose-internals@4.9.4",
60+
"@types/ts-expose-internals": "npm:ts-expose-internals@5.1.3",
6161
"@types/validate-npm-package-name": "^4.0.0"
6262
},
6363
"volta": {

‎packages/core/src/checks/resolutionBasedFileProblems.ts

Lines changed: 49 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,18 @@ export function getResolutionBasedFileProblems(
1010
): ResolutionBasedFileProblem[] {
1111
const result: ResolutionBasedFileProblem[] = [];
1212
for (const resolutionOption of allResolutionOptions) {
13-
const visibleFiles = Object.values(entrypointResolutions).flatMap((entrypoint) => {
14-
const files = new Set<string>();
15-
getResolutionKinds(resolutionOption).forEach((resolutionKind) => {
16-
entrypoint.resolutions[resolutionKind].files?.forEach((file) => files.add(file));
17-
if (entrypoint.resolutions[resolutionKind].implementationResolution) {
18-
files.add(entrypoint.resolutions[resolutionKind].implementationResolution!.fileName);
19-
}
20-
});
21-
return Array.from(files);
22-
});
13+
const visibleFiles = new Set(
14+
Object.values(entrypointResolutions).flatMap((entrypoint) => {
15+
const files = new Set<string>();
16+
getResolutionKinds(resolutionOption).forEach((resolutionKind) => {
17+
entrypoint.resolutions[resolutionKind].files?.forEach((file) => files.add(file));
18+
if (entrypoint.resolutions[resolutionKind].implementationResolution) {
19+
files.add(entrypoint.resolutions[resolutionKind].implementationResolution!.fileName);
20+
}
21+
});
22+
return Array.from(files);
23+
})
24+
);
2325

2426
for (const fileName of visibleFiles) {
2527
const sourceFile = host.getSourceFile(fileName, resolutionOption)!;
@@ -46,13 +48,11 @@ export function getResolutionBasedFileProblems(
4648
kind: "InternalResolutionError",
4749
resolutionOption,
4850
fileName,
49-
error: {
50-
moduleSpecifier: reference,
51-
pos: moduleSpecifier.pos,
52-
end: moduleSpecifier.end,
53-
resolutionMode,
54-
trace: host.getTrace(resolutionOption, fileName, moduleSpecifier.text, resolutionMode)!,
55-
},
51+
moduleSpecifier: reference,
52+
pos: moduleSpecifier.pos,
53+
end: moduleSpecifier.end,
54+
resolutionMode,
55+
trace: host.getTrace(resolutionOption, fileName, moduleSpecifier.text, resolutionMode)!,
5656
});
5757
}
5858
}
@@ -62,34 +62,38 @@ export function getResolutionBasedFileProblems(
6262
// try to do a ts->js extension substitution and assume that's a
6363
// visible JS file if it exists.
6464
//
65-
// TODO: this should maybe only be issued in node16
66-
if (ts.hasJSFileExtension(fileName)) {
67-
const expectedModuleKind = host.getModuleKindForFile(fileName, resolutionOption);
68-
const syntaxImpliedModuleKind = sourceFile.externalModuleIndicator
69-
? ts.ModuleKind.ESNext
70-
: sourceFile.commonJsModuleIndicator
71-
? ts.ModuleKind.CommonJS
72-
: undefined;
73-
if (
74-
expectedModuleKind !== undefined &&
75-
syntaxImpliedModuleKind !== undefined &&
76-
expectedModuleKind.detectedKind !== syntaxImpliedModuleKind
77-
) {
78-
const syntax = sourceFile.externalModuleIndicator ?? sourceFile.commonJsModuleIndicator;
79-
result.push({
80-
kind: "UnexpectedModuleSyntax",
81-
resolutionOption,
82-
syntax: syntaxImpliedModuleKind,
83-
fileName,
84-
range:
85-
typeof syntax === "object"
86-
? {
87-
pos: syntax.getStart(sourceFile),
88-
end: syntax.end,
89-
}
90-
: undefined,
91-
moduleKind: expectedModuleKind,
92-
});
65+
// Actually, we probably want to check the relative directory relationship
66+
// between an entrypoint resolution and implementationResolution as the basis
67+
// for looking for a JS file.
68+
if (resolutionOption === "node16") {
69+
if (ts.hasJSFileExtension(fileName)) {
70+
const expectedModuleKind = host.getModuleKindForFile(fileName, resolutionOption);
71+
const syntaxImpliedModuleKind = sourceFile.externalModuleIndicator
72+
? ts.ModuleKind.ESNext
73+
: sourceFile.commonJsModuleIndicator
74+
? ts.ModuleKind.CommonJS
75+
: undefined;
76+
if (
77+
expectedModuleKind !== undefined &&
78+
syntaxImpliedModuleKind !== undefined &&
79+
expectedModuleKind.detectedKind !== syntaxImpliedModuleKind
80+
) {
81+
const syntax = sourceFile.externalModuleIndicator ?? sourceFile.commonJsModuleIndicator;
82+
result.push({
83+
kind: "UnexpectedModuleSyntax",
84+
resolutionOption,
85+
syntax: syntaxImpliedModuleKind,
86+
fileName,
87+
range:
88+
typeof syntax === "object"
89+
? {
90+
pos: syntax.getStart(sourceFile),
91+
end: syntax.end,
92+
}
93+
: undefined,
94+
moduleKind: expectedModuleKind,
95+
});
96+
}
9397
}
9498
}
9599
}

‎packages/core/src/multiCompilerHost.ts

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ export function createMultiCompilerHost(fs: FS): MultiCompilerHost {
5959
traceResolution: true,
6060
},
6161
bundler: {
62-
// @ts-expect-error
6362
moduleResolution: ts.ModuleResolutionKind.Bundler,
6463
module: ts.ModuleKind.ESNext,
6564
target: ts.ScriptTarget.Latest,
@@ -182,15 +181,22 @@ export function createMultiCompilerHost(fs: FS): MultiCompilerHost {
182181
);
183182
const trace = traceCollector.read();
184183
const moduleKey = `${resolutionMode ?? 1}:${moduleName}`;
185-
(traceCache[moduleResolution][containingFile] ??= {})[moduleKey] = trace;
184+
if (!traceCache[moduleResolution][containingFile]?.[moduleKey]) {
185+
(traceCache[moduleResolution][containingFile] ??= {})[moduleKey] = trace;
186+
}
186187
return {
187188
resolution,
188189
trace,
189190
};
190191
}
191192

192-
function getTrace(moduleResolution: ResolutionOption, fromFileName: string, key: string): string[] | undefined {
193-
return traceCache[moduleResolution][fromFileName]?.[key];
193+
function getTrace(
194+
moduleResolution: ResolutionOption,
195+
fromFileName: string,
196+
moduleSpecifier: string,
197+
resolutionMode: ts.ModuleKind.ESNext | ts.ModuleKind.CommonJS
198+
): string[] | undefined {
199+
return traceCache[moduleResolution][fromFileName]?.[`${resolutionMode ?? 1}:${moduleSpecifier}`];
194200
}
195201

196202
function createProgram(moduleResolution: ResolutionOption, rootNames: string[]): ts.Program {
@@ -231,6 +237,18 @@ export function createMultiCompilerHost(fs: FS): MultiCompilerHost {
231237
useCaseSensitiveFileNames,
232238
getNewLine,
233239
trace: traceCollector.trace,
240+
resolveModuleNameLiterals(moduleLiterals, containingFile, _redirectedReference, options, containingSourceFile) {
241+
return moduleLiterals.map(
242+
(literal) =>
243+
resolveModuleName(
244+
literal.text,
245+
containingFile,
246+
moduleResolution,
247+
ts.getModeForUsageLocation(containingSourceFile, literal),
248+
options.noDtsResolution
249+
).resolution
250+
);
251+
},
234252
};
235253
}
236254

0 commit comments

Comments
 (0)