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

Consistently avoid module resolution errors when using getSymbolAtLocation #58668

Merged

Conversation

Andarist
Copy link
Contributor

@Andarist Andarist commented May 27, 2024

fixes #58600
fixes #58791

@typescript-bot typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label May 27, 2024
@@ -4521,17 +4521,17 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const errorMessage = isClassic ?
Diagnostics.Cannot_find_module_0_Did_you_mean_to_set_the_moduleResolution_option_to_nodenext_or_to_add_aliases_to_the_paths_option
: Diagnostics.Cannot_find_module_0_or_its_corresponding_type_declarations;
return resolveExternalModuleNameWorker(location, moduleReferenceExpression, ignoreErrors ? undefined : errorMessage);
return resolveExternalModuleNameWorker(location, moduleReferenceExpression, ignoreErrors ? undefined : errorMessage, ignoreErrors);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the core of the fix. It propagates the ignoreErrors and the intention to ignore them is highlighted in the comment here:
https://github.dev/microsoft/TypeScript/blob/af3a61fe4487a92d59f9479aa4249d897b91af14/src/compiler/checker.ts#L1654-L1658

src/compiler/checker.ts Outdated Show resolved Hide resolved
@@ -114,4 +114,58 @@ describe("unittests:: tsc:: composite::", () => {
},
],
});

verifyTsc({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

those are tests related to the referenced issue

Comment on lines 111 to 115
"referencedMap": {
"./main.ts": [
"./data.d.json.ts"
]
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This feels like an improvement - even though there is a resolution error here, it still depends on this file so it should be reflected here. IIRC, there are other instances of something like this - when despite a resolution errors requested modules are listed here.

Comment on lines -50 to -53
b.ts:1:8 - error TS5097: An import path can only end with a '.ts' extension when 'allowImportingTsExtensions' is enabled.

1 import "./a.ts";
   ~~~~~~~~

[HH:MM:SS AM] Found 1 error. Watching for file changes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this error should be reported but currently it was reported here accidentally. It's not consistent with a regular - non-incremental/composite executions. I raised an issue about it here: #58725

Copy link
Member

Choose a reason for hiding this comment

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

FWIW resolving these would be #50394

@Andarist Andarist force-pushed the fix/crash-error-on-synthetic-import branch from fe293a5 to 039a66e Compare June 7, 2024 07:20
@jakebailey
Copy link
Member

@typescript-bot test it
@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jun 7, 2024

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
test top400 ✅ Started ✅ Results
user test this ✅ Started ✅ Results
run dt ✅ Started ✅ Results
perf test this faster ✅ Started 👀 Results
pack this ✅ Started ✅ Results

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jun 7, 2024

Hey @jakebailey, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/162131/artifacts?artifactName=tgz&fileId=71D5E87C0254083AD9A25080B19F958EEA359780CCE198BDB12841AA1D3452D702&fileName=/typescript-5.6.0-insiders.20240607.tgz"
    }
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@5.6.0-pr-58668-2".;

@typescript-bot
Copy link
Collaborator

Hey @jakebailey, the results of running the DT tests are ready.

Everything looks the same!

You can check the log here.

@typescript-bot
Copy link
Collaborator

@jakebailey Here are the results of running the user tests with tsc comparing main and refs/pull/58668/merge:

Everything looks good!

@typescript-bot
Copy link
Collaborator

@jakebailey
The results of the perf run you requested are in!

Here they are:

tsc

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Compiler-Unions - node (v18.15.0, x64)
Errors 30 30 ~ ~ ~ p=1.000 n=6
Symbols 62,153 62,153 ~ ~ ~ p=1.000 n=6
Types 50,242 50,242 ~ ~ ~ p=1.000 n=6
Memory used 192,948k (± 0.79%) 193,600k (± 0.91%) ~ 192,343k 195,863k p=0.173 n=6
Parse Time 1.30s (± 0.90%) 1.30s (± 0.58%) ~ 1.29s 1.31s p=0.933 n=6
Bind Time 0.72s 0.72s ~ ~ ~ p=1.000 n=6
Check Time 9.57s (± 0.21%) 9.60s (± 0.47%) ~ 9.53s 9.65s p=0.164 n=6
Emit Time 2.75s (± 0.82%) 2.76s (± 1.04%) ~ 2.73s 2.80s p=0.685 n=6
Total Time 14.34s (± 0.16%) 14.38s (± 0.28%) ~ 14.32s 14.42s p=0.119 n=6
angular-1 - node (v18.15.0, x64)
Errors 5 5 ~ ~ ~ p=1.000 n=6
Symbols 944,109 944,109 ~ ~ ~ p=1.000 n=6
Types 407,049 407,049 ~ ~ ~ p=1.000 n=6
Memory used 1,222,237k (± 0.01%) 1,222,246k (± 0.00%) ~ 1,222,198k 1,222,341k p=0.936 n=6
Parse Time 6.79s (± 0.57%) 6.78s (± 0.57%) ~ 6.72s 6.83s p=0.687 n=6
Bind Time 1.87s (± 0.40%) 1.88s (± 0.22%) ~ 1.87s 1.88s p=0.100 n=6
Check Time 31.29s (± 0.36%) 31.25s (± 0.23%) ~ 31.15s 31.37s p=0.688 n=6
Emit Time 15.22s (± 0.53%) 15.24s (± 0.84%) ~ 15.03s 15.35s p=0.575 n=6
Total Time 55.18s (± 0.31%) 55.15s (± 0.30%) ~ 54.94s 55.38s p=0.688 n=6
mui-docs - node (v18.15.0, x64)
Errors 5 5 ~ ~ ~ p=1.000 n=6
Symbols 2,063,474 2,063,474 ~ ~ ~ p=1.000 n=6
Types 905,299 905,299 ~ ~ ~ p=1.000 n=6
Memory used 2,072,777k (± 0.01%) 2,072,760k (± 0.01%) ~ 2,072,576k 2,072,908k p=0.630 n=6
Parse Time 8.10s (± 0.35%) 8.11s (± 0.28%) ~ 8.09s 8.15s p=0.685 n=6
Bind Time 2.76s (± 0.91%) 2.75s (± 0.81%) ~ 2.72s 2.78s p=0.805 n=6
Check Time 82.47s (± 0.40%) 82.49s (± 0.56%) ~ 82.08s 83.13s p=0.810 n=6
Emit Time 0.16s (± 3.16%) 0.16s (± 3.16%) ~ 0.16s 0.17s p=1.000 n=6
Total Time 93.50s (± 0.36%) 93.52s (± 0.53%) ~ 93.06s 94.22s p=0.689 n=6
self-build-src - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,230,424 1,230,497 +73 (+ 0.01%) ~ ~ p=0.001 n=6
Types 261,178 261,204 +26 (+ 0.01%) ~ ~ p=0.001 n=6
Memory used 2,351,921k (± 0.04%) 2,351,800k (± 0.04%) ~ 2,350,729k 2,353,077k p=0.936 n=6
Parse Time 4.97s (± 0.95%) 4.98s (± 0.91%) ~ 4.95s 5.07s p=0.810 n=6
Bind Time 1.90s (± 0.40%) 1.89s (± 0.67%) ~ 1.87s 1.90s p=0.301 n=6
Check Time 34.15s (± 0.29%) 34.03s (± 0.33%) ~ 33.89s 34.15s p=0.173 n=6
Emit Time 2.61s (± 1.72%) 2.62s (± 3.04%) ~ 2.53s 2.75s p=0.810 n=6
Total Time 43.64s (± 0.17%) 43.53s (± 0.39%) ~ 43.38s 43.84s p=0.093 n=6
self-build-src-public-api - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,230,424 1,230,497 +73 (+ 0.01%) ~ ~ p=0.001 n=6
Types 261,178 261,204 +26 (+ 0.01%) ~ ~ p=0.001 n=6
Memory used 2,427,402k (± 0.05%) 2,427,814k (± 0.02%) ~ 2,427,190k 2,428,580k p=0.471 n=6
Parse Time 5.20s (± 0.95%) 5.19s (± 1.58%) ~ 5.09s 5.30s p=0.748 n=6
Bind Time 1.69s (± 0.79%) 1.68s (± 0.70%) ~ 1.66s 1.69s p=0.241 n=6
Check Time 34.58s (± 0.13%) 34.51s (± 0.39%) ~ 34.33s 34.69s p=0.378 n=6
Emit Time 2.67s (± 3.04%) 2.67s (± 0.64%) ~ 2.65s 2.70s p=1.000 n=6
Total Time 44.15s (± 0.27%) 44.06s (± 0.42%) ~ 43.82s 44.30s p=0.378 n=6
self-compiler - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 259,181 259,182 +1 (+ 0.00%) ~ ~ p=0.001 n=6
Types 105,173 105,173 ~ ~ ~ p=1.000 n=6
Memory used 428,839k (± 0.01%) 428,815k (± 0.01%) ~ 428,781k 428,843k p=0.230 n=6
Parse Time 4.10s (± 0.62%) 4.06s (± 0.43%) -0.04s (- 0.85%) 4.04s 4.09s p=0.034 n=6
Bind Time 1.63s (± 1.43%) 1.63s (± 1.39%) ~ 1.60s 1.66s p=0.568 n=6
Check Time 22.36s (± 0.35%) 22.36s (± 0.35%) ~ 22.27s 22.46s p=0.872 n=6
Emit Time 1.72s (± 1.68%) 1.72s (± 1.74%) ~ 1.68s 1.76s p=0.871 n=6
Total Time 29.80s (± 0.31%) 29.77s (± 0.26%) ~ 29.65s 29.84s p=0.421 n=6
ts-pre-modules - node (v18.15.0, x64)
Errors 35 35 ~ ~ ~ p=1.000 n=6
Symbols 224,565 224,565 ~ ~ ~ p=1.000 n=6
Types 93,734 93,734 ~ ~ ~ p=1.000 n=6
Memory used 369,754k (± 0.02%) 369,820k (± 0.03%) ~ 369,637k 369,925k p=0.298 n=6
Parse Time 3.45s (± 0.81%) 3.45s (± 0.68%) ~ 3.42s 3.49s p=0.570 n=6
Bind Time 1.94s (± 0.98%) 1.94s (± 0.84%) ~ 1.92s 1.96s p=0.744 n=6
Check Time 19.35s (± 0.31%) 19.42s (± 0.28%) ~ 19.34s 19.49s p=0.077 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 24.75s (± 0.35%) 24.80s (± 0.25%) ~ 24.75s 24.89s p=0.335 n=6
vscode - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 2,851,240 2,851,240 ~ ~ ~ p=1.000 n=6
Types 967,181 967,181 ~ ~ ~ p=1.000 n=6
Memory used 3,023,304k (± 0.00%) 3,023,299k (± 0.00%) ~ 3,023,247k 3,023,394k p=0.689 n=6
Parse Time 13.96s (± 0.29%) 13.92s (± 0.24%) ~ 13.88s 13.97s p=0.126 n=6
Bind Time 4.17s (± 0.24%) 4.16s (± 0.29%) ~ 4.15s 4.18s p=0.490 n=6
Check Time 76.71s (± 2.67%) 75.31s (± 2.46%) ~ 73.80s 77.73s p=0.230 n=6
Emit Time 21.20s (± 8.72%) 22.51s (± 8.25%) ~ 20.11s 24.01s p=0.093 n=6
Total Time 116.03s (± 0.35%) 115.90s (± 0.21%) ~ 115.67s 116.34s p=0.810 n=6
webpack - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 266,415 266,415 ~ ~ ~ p=1.000 n=6
Types 108,580 108,580 ~ ~ ~ p=1.000 n=6
Memory used 411,445k (± 0.01%) 411,426k (± 0.01%) ~ 411,371k 411,474k p=0.689 n=6
Parse Time 3.87s (± 0.67%) 3.84s (± 0.77%) ~ 3.80s 3.87s p=0.106 n=6
Bind Time 1.67s (± 1.03%) 1.67s (± 0.72%) ~ 1.66s 1.69s p=0.934 n=6
Check Time 17.08s (± 0.27%) 17.06s (± 0.21%) ~ 17.03s 17.12s p=0.296 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 22.62s (± 0.28%) 22.57s (± 0.15%) ~ 22.53s 22.62s p=0.146 n=6
xstate-main - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 526,436 526,436 ~ ~ ~ p=1.000 n=6
Types 179,425 179,425 ~ ~ ~ p=1.000 n=6
Memory used 464,208k (± 0.04%) 464,080k (± 0.08%) ~ 463,585k 464,453k p=0.810 n=6
Parse Time 3.13s (± 0.77%) 3.15s (± 0.58%) ~ 3.13s 3.17s p=0.252 n=6
Bind Time 1.18s (± 0.46%) 1.19s (± 0.82%) ~ 1.18s 1.21s p=0.201 n=6
Check Time 18.10s (± 0.60%) 18.08s (± 0.53%) ~ 17.89s 18.14s p=0.470 n=6
Emit Time 0.00s 0.00s (±244.70%) ~ 0.00s 0.01s p=0.405 n=6
Total Time 22.42s (± 0.42%) 22.42s (± 0.39%) ~ 22.25s 22.49s p=1.000 n=6
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • Compiler-Unions - node (v18.15.0, x64)
  • angular-1 - node (v18.15.0, x64)
  • mui-docs - node (v18.15.0, x64)
  • self-build-src - node (v18.15.0, x64)
  • self-build-src-public-api - node (v18.15.0, x64)
  • self-compiler - node (v18.15.0, x64)
  • ts-pre-modules - node (v18.15.0, x64)
  • vscode - node (v18.15.0, x64)
  • webpack - node (v18.15.0, x64)
  • xstate-main - node (v18.15.0, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

Developer Information:

Download Benchmarks

@typescript-bot
Copy link
Collaborator

@jakebailey Here are the results of running the top 400 repos with tsc comparing main and refs/pull/58668/merge:

Everything looks good!

@jakebailey
Copy link
Member

This appears to be out of date with main.

@jakebailey jakebailey merged commit 3743fbc into microsoft:main Jun 24, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
7 participants