microsoft / TypeScript

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

Go to Definition for Non JS/TS File Imports #15146

Open mjbvz opened 7 years ago

mjbvz commented 7 years ago

From https://github.com/Microsoft/vscode/issues/24276

Issue Users of tools such as webpack often load non js/ts assets, such as css or images, using require or import:

require('./main.css');
import './image.png'
...

Operations such as go to definition currently do not work on these paths

Proposal go to definition on these resource paths should try to open the corresponding resource in the editor.

aluanhaddad commented 7 years ago

They're a couple of great VS Code plugins that do this at the moment. I assume they work by returning a "definition" that maps to a file and that VS Code then seems adds this as a possible definition to navigate to (I don't really know if this is how it works, but this is how it appears).

Because of this, they suffer from the following problem:

Say I have a declaration file, let's call it ambiance.d.ts with the following content

declare module '*.css' { export default '' as string; }
declare module '*.scss' { export default '' as string; }
declare module '*.html' { export default '' as string; }

in this case, despite of the plugins efforts, the user always sees a list of two definitions to go to instead of being able to jump directly to the file.

That is really the only drawback to the current plugins.

Having said that, I always thought it would be nice if TypeScript could validate the existence of these asset files but i think that ship has long ago sailed.

mhegazy commented 7 years ago

My main concern is that this is not ES6 compliant. ES6 does not talk about importing non-code modules. So this code is not really valid ES6 and will not work with a native ES6 module implementation.

So we can pretend that it will work, and just let it pass. or leave it the way it is and be spec-compliant.

mjbvz commented 7 years ago

I'm OK with not including this as builtin TS functionality given the compliance concerns. Is there any way we can leverage TSServer for these scenarios however? Perhaps with a TSServer plugin?

mhegazy commented 7 years ago

This is part of the module resolution logic, so we have to add it in the core.

thw0rted commented 7 years ago

Since this is specific to Webpack's implementation of require(), I would argue that it should go in @types/webpack-env, not core TypeScript. Isn't that how this is supposed to work?

aluanhaddad commented 7 years ago

@thw0rted how is this specific to Webpack?

thw0rted commented 7 years ago

I should have said, since it's specific to the loader / bundler's implementation of require(). I was under the impression that TS didn't provide a native capability to require text resources, but maybe I misunderstood.

aluanhaddad commented 7 years ago

@thw0rted

I should have said, since it's specific to the loader / bundler's implementation of require(). I was under the impression that TS didn't provide a native capability to require text resources, but maybe I misunderstood.

It is correct that TypeScript does not provide this capability, but Webpack is just one of many tools that do.

thw0rted commented 7 years ago

I think my point stands, in that case -- nothing in core TypeScript provides for require("foo.png") (or css or html or whatever) so it shouldn't be in an ambient declaration unless I've specifically added typings for an environment that does provide that ability. Right?

aluanhaddad commented 7 years ago

Yes you are quite correct. I didn't mean to suggest otherwise I was just mentioning that the notion is more general than Webpack.

jpike88 commented 6 years ago

Additionally TypeScript should be checking the path of EVERY import that occurs (module or otherwise) and raise an appropriate error. A hacky workaround silences an error that an html file isn't a module, but an incorrect relative path doesn't re-flag an error. So I'm stuck guessing that my import paths are hopefully correct.

https://github.com/Microsoft/vscode/issues/48025#issuecomment-382623046

I don't get the argument regarding strict ES6 spec compliance. This is something webpack, fusebox and pretty much every bundler out there allows and encourages. I'd imagine this has plenty of devs out there using it. Widespread real-world use cases matter, so the meaning of what is and isn't 'compliant' needs to be re-evaluated in this case.

Besides if this needs to be supported by the core, and maybe just have an appropriate .tsconfig flag turned off by default, why not just do it?

thw0rted commented 6 years ago

I think the argument is that Typescript never has to actually load the file contents -- that's handled by your bundler, after transpilation. require and import are provided by webpack or gulp or whatever, so for tsc to process these correctly, they'd have to re-implement those programs' loading logic. What if your require operates differently based on the resolve options in your webpack.config.js?

I think it's important to address exactly what we're trying to solve. Is it edit-time path checking, so you get a red squiggle in VS Code when the file doesn't exist? Or is it transpile-time checking so that tsc flags a non-existent require, even though tsc will never actually read the contents of that file? If it's the former, as comments upthread point out, VS Code has plugins that do this. If it's the latter, aren't you just going to pass the generated JS through a bundler as soon as tsc completes? What's the point of extra checking, in that case?

jpike88 commented 6 years ago

Because Typescript doesn't need to load the file contents of non-code imports, a simple path check would be trivial to implement... otherwise every person that uses Typescript + a bundler would need an extra plugin for smooth usage. Considering how ubiquitous bundlers have become, I still think this would be a trivial addition to the Typescript functionality set...

As for a plugin that does the above, I'm having a hard time finding one. Can someone suggest one?

shinriyo commented 6 years ago

VSCode never supports?

Or, I want plugin or I'll develop myself

octref commented 6 years ago

Just a quick note here: I implemented CSS/SCSS's @import using DocumentLink as compared to using Definition (both can be cmd-clicked for going to another location). I believe it should be possible to add a ts server plugin that generates those DocumentLink for text documents.

dwelle commented 5 years ago

Note that this doesn't work even for importing json files which are AFAIK in es6 module spec.

mjbvz commented 5 years ago

@dwelle See the resolveJsonModule setting added in TS 2.9: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-9.html

dwelle commented 5 years ago

@mjbvz can't seem to make it work, using this jsconfig.json:

{
    "compilerOptions": {
        "rootDir": ".",
        "module": "system",
        "resolveJsonModule": true
    },
    /* ... */
}
dwelle commented 5 years ago

Ok, it seems that resolveJsonModule config requires module: "es6" (though the error says it needs node module, which isn't valid --- I'd assume a commonjs would qualify as node, but the json resolution doesn't work with that). But es6 doesn't work with absolute paths (using the rootDir: ".").

So I guess I can either choose between absolute paths, or json path resolution, not both. It does make sense since the json resolution is part of es6, not commonjs, but as this issue says, I'm not sure that a text editor should be this pedantic. Better if it just works.

sbussard commented 4 years ago

This issue has been bugging me a lot. I'm glad someone else brought it up first.

ahaq0 commented 4 years ago

Currently, what's the best workaround?

Lexicality commented 4 years ago

I think it's important to address exactly what we're trying to solve.

The problems my team and I currently have are:

Would something like a supplimentalExtensions config value work for this? It would then entirely be up to the user to make sure the module resolution system is the same between Webpack and Typescript.

(Presumably the typescript compiler would emit an error if asked to bundle a file with a non-js import)

joshglenn commented 4 years ago

I had read this issue earlier today, before I filed issue 90155 in vscode . This does not appear to me to address my problem.

But if, as you said earlier in this thread, there are extensions that workaround this I'd sure like to know of one. I've been scouring the extensions gallery trying to find it.

Do you know any by name that I could try?

DMQ commented 4 years ago

I found a vscode plugin called file peek to provide this functionality. Hope also can help you guys.

Cpanmac commented 4 years ago

When can we deal with this problem?@mjbvz

mishannn commented 4 years ago

It is very important to autocomplete files other than JS or TS and check the existence of a file. Due to its absence, typos often appear and, in general, work becomes less comfortable. Please make this opportunity.

FrederickEngelhardt commented 3 years ago

Bump. It would be nice to have a way to do file path resolution with wild cards and extrapolate the beginning path and resolve the file using node.

declare module '*.css?raw' {
  const content: { [className: string]: string }
  file = require('[filepath].css')

  export default content
  return file
}

The above would both declare the typing and return the file it should be resolving. Of course there would need to be escape hatches for bad paths etc.

edouard-lopez commented 3 years ago

In the meantine, use can leverage CSS Navigation extension className resolution.

It planned to support import resolution in a future release.

dummdidumm commented 3 years ago

I think this can be closed since this landed in TS 4.3 https://devblogs.microsoft.com/typescript/announcing-typescript-4-3/#go-to-def-non-js

Yegorich555 commented 3 years ago

I think this can be closed since this landed in TS 4.3 https://devblogs.microsoft.com/typescript/announcing-typescript-4-3/#go-to-def-non-js

Yep. I've just rechecked. Looks lite it works now

chkpnt commented 2 years ago

Yes, go to definition works. But how can I get rid of the warning "Cannot find module '../img/bla.png' or its corresponding type declarations.ts(2307)" while preserving this feature?

If I declare module '*.png' in global.d.ts the warning disappears, but then I'm again unable to use go-to-definition.

NerdPraise commented 2 years ago

@chkpnt A workaround; not too good but it works declare module '*.png' { export default '' as any; }

Removes the warning and also go-to-definition

chkpnt commented 2 years ago

Unfortunately, adding { export default '' as any; } doesn't help. When I use go-to-definition, I end up in global.d.ts (which might be right, as there is the definition), but I want to open the file I'm importing. 🤷‍♂️

aleclarson commented 3 months ago

Here's an extension that implements the requested feature: https://marketplace.visualstudio.com/items?itemName=thisismanta.third-eye

shankarThiyagaraajan commented 2 months ago

Here's an extension that implements the requested feature: https://marketplace.visualstudio.com/items?itemName=thisismanta.third-eye

It’s not solving the '.scss' navigation to file and style class. Issue still exist.