microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.75k stars 12.46k forks source link

Go to definition broken by existance of tsconfig.json in JavaScript project. #40978

Open Raynos opened 4 years ago

Raynos commented 4 years ago

TypeScript Version: 4.0.2

Search Terms: go to definition javascript

Code

'use strict'

// javascript file.
const uuid = require('uuid')

Expected behavior:

When I hit Go to definition with F12 on uuid it goes to the source code.

working-go-to-def

This works fine for JavaScript because tsconfig.json does not exist.

Actual behavior:

When I hit Go to definition with F12 it does not do anything useful.

broken-go-todef

This is because a tsconfig.json file exists in my JavaScript project.

Playground Link:

Related Issues:

Raynos commented 4 years ago

I renamed the tsconfig.json to jsconfig.json and now go to definition works again ...

I guess it's my fault for not using jsconfig.json ...

andrewbranch commented 4 years ago

I believe this is because when we see a tsconfig.json, we have a default of maxNodeModuleJsDepth: 0 since we prefer you to have type declaration files for all the modules you use (because if authored correctly, they’re more accurate and waaay cheaper to process). I agree this is a bad consequence of that logic, though. We could plausibly resolve to the JS file on-demand if we don’t see any types... my only concern is that for a TypeScript user, that “no definition found” message can be a useful cue to install @types/uuid. I think we would want the editor to somehow preserve the implication that there is missing information, even as we do the best we can with what exists. @mjbvz @DanielRosenwasser, any thoughts?

Raynos commented 4 years ago

my only concern is that for a TypeScript user, that “no definition found” message can be a useful cue to install @types/uuid

This makes sense to me if I'm editing a .ts typescript file and I'm using the typescript import syntax.

It was not clear from my example but it's actually a .js file and I'm using require because my version of node does not support import / export.

Raynos commented 4 years ago

maxNodeModuleJsDepth

This is an interesting point. I've noticed the consequence of using jsconfig.json in that I need to set maxNodeModuleJsDepth manually with the cli like tsc -p jsconfig.json --maxNodeModuleJsDepth 0

I do want this max depth 0 behavior when running the compiler to check my code, but i want >0 max depth when using go to definition on a require statement in a javascript file.

andrewbranch commented 4 years ago

This makes sense to me if I'm editing a .ts typescript file and I'm using the typescript import syntax.

Well, if we see JavaScript governed by a tsconfig file, we come pretty close to treating it like TypeScript. I have an open source project I maintain in JavaScript with JSDoc that I check with the TypeScript compiler, and I understand that require statements are treated the same as imports in TS, and if I see that one of my requires doesn’t magically get a go-to-definition on it, it makes me realize that I should try to install the @types package for it. But overall I think you’re right; we should be willing to jump from JS to JS in the editor, even if we wouldn’t consume that JS as part of type checking. I just want to avoid confusing TS-savvy JS users who are used to go-to-definition implying the presence of type info in the program.

Raynos commented 3 years ago

I just want to avoid confusing TS-savvy JS users who are used to go-to-definition implying the presence of type info in the program.

image

We have "go to type definition" in VS code. Why can TS-savvy users not use "Go to type definition" and leave "Go to definition" to mean to go the definition of the function/class/object implementation, wherever that happens to be on disk ?

andrewbranch commented 3 years ago

That’s #6209

Raynos commented 3 years ago

That’s a .ts file specific issue. Go to definition already behaves like go to implementation for javascript files.

andrewbranch commented 3 years ago

It’s not specific to TS files. You can only jump to a JS definition if that JS is what the compiler analyzed to get types for the symbol you’re inspecting. In a JS project, this will be the case for your own source files, and for node_modules that don’t have typings, but not for node_modules that do have typings (whether provided alongside the JS or via @types). Anyway, my point is that the reason we can’t just solve this by bifurcating the behavior of go-to-definition and go-to-type-definition is that the behavior you want for the former doesn’t exist, and is tracked by #6209.