Typescript: Give better error messages for bare imports with badly configured projects

[DanielRosenwasser]

When targeting es2015 or later, moduleResolution isn't set to node by default.

Perhaps when

• moduleResolution is calculated to be classic and
• paths aren't specified

we should hint to the user

Did you mean to set the 'moduleResolution' option to 'node', or to add aliases to the 'paths' option?

[FredyC]

I want to add here, that the Playground does not even allow to set a moduleResolution.

I tried reproducing some errors and was confused about why am I getting different outcomes in the playground compared to my actual code.

Only after fiddling with it CodeSandbox I eventually found out that moduleResolution: node did the trick.

I was not able to find if Playground issues should be reported somewhere else. Should I open a separate issue here?

Why is the classic resolution still being kept as default for a new module types?

[DanielRosenwasser]

Yes, https://github.com/Microsoft/TypeScript-Website, though I would say the website should probably just always display moduleResolution: node since anything else is useless.

Why is the classic resolution still being kept as default for a new module types?

Yeah, there's https://github.com/microsoft/TypeScript/issues/11434 but I don't think we ever got around to the work.

[orta]

The playground side of this is done (it always defaults to moduleResolution: node. )

Which leaves the warning on imports to do 👍

[mshivaku99]

Can me and @nityatammana work on this?

[DanielRosenwasser]

Sure - you might want to create one new diagnostic in diagnosticMessages.json

Cannot find module '{0}'. Did you mean to set the 'moduleResolution' option to 'node', or to add aliases to the 'paths' option?

Run npx gulp generate-diagnostics

Run the tests

npx gulp runtests-parallel

see what breaks. Diff the folders tests/baselines/local and tests/baselines/reference with a tool like WinMerge or Beyond Compare to see if the new errors are good.

If nothing fails, you need to add a test case in a file named tests/cases/compiler/errorForBareSpecifierWithImplicitModuleResolution1.ts:

// @module: es2015

import { thing } from "non-existent-module";
thing()

Then run the tests to get baselines from that. Accept the baselines:

npx gulp baseline-accept

[mshivaku99]

Hi again! To check in we've added the diagnostic and ran npx gulp generate diagnostics and npx runtests-parallel (on Windows through WSL). This was successful and it seemed as if the whole test suite passed. However, when diff-ing the folders the local folder just had the directories and subdirectories but no files.

Local Folder

Reference Folder

We're confused as to how to populate the local folder beyond running the tests locally.

Also, to clarify, in addition to adding to diagnosticMessages.json and adding test cases, do we need to throw the error in the source code?

[orta]

The local folder only changes when the baselines have changed - so if your diffs don't match the reference folder.

Adding the throw in the user's project (e.g. creating the error diagnostic) should be the thing which will break a reference test of two hopefully.

[DanielRosenwasser]

Let's steer away from the word "throw" because that's just not the way TypeScript reports errors.

You need to report the diagnostic/error that I mentioned in my last comment instead of the current error when the project is misconfigured.

[mshivaku99]

Me and @nityatammana think we've found a fix, but after accepting the baselines, 10 tests fail that don't include tests we've modified baselines for and don't appear in the tests/baselines/local folder after failing. I've attached images of the first 3 below.

If anyone has any insights, I would be very happy to hear it!

[sheetalkamat]

You need to edit the error on src\testRunner\unittests\tsserver\symLinks.ts

[tangobravo]

If the default isn't changed to 'node' everywhere (to maintain backwards compatibility) - could a warning just be emitted whenever 'moduleResolution' defaults to classic (rather than just when an import fails).

Something like "warning: moduleResolution defaulting to classic due to your module setting, which is probably not what you want."

If the moduleResolution is explicitly set to classic (either in tsconfig or cmd line) then that would silence the warning.

I'd personally favour just changing the default, but as it would be a behaviour change for existing projects then I understand why it might not have happened yet.

[DanielRosenwasser]

Thanks for the PR @mshivaku99, and thank you @orta for helping guide the PR!

[DanielRosenwasser]

(and @sheetalkamat for the guidance above!)