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

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

Open
mjbvz opened this issue Apr 12, 2017 · 35 comments
Open

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

mjbvz opened this issue Apr 12, 2017 · 35 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Domain: JavaScript The issue relates to JavaScript specifically Domain: Symbol Navigation Relates to go-to-definition, find-all-references, highlighting/occurrences. Suggestion An idea for TypeScript VS Code Tracked There is a VS Code equivalent to this issue

Comments

@mjbvz
Copy link
Contributor

mjbvz commented Apr 12, 2017

From microsoft/vscode#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
Copy link
Contributor

aluanhaddad commented Apr 12, 2017

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
Copy link
Contributor

mhegazy commented Apr 19, 2017

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.

@mhegazy mhegazy added Suggestion An idea for TypeScript Salsa Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature labels Apr 19, 2017
@mjbvz
Copy link
Contributor Author

mjbvz commented Apr 19, 2017

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
Copy link
Contributor

mhegazy commented Apr 19, 2017

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

@thw0rted
Copy link

thw0rted commented Aug 3, 2017

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
Copy link
Contributor

@thw0rted how is this specific to Webpack?

@thw0rted
Copy link

thw0rted commented Aug 6, 2017

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
Copy link
Contributor

@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
Copy link

thw0rted commented Aug 7, 2017

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
Copy link
Contributor

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
Copy link

jpike88 commented Apr 19, 2018

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.

microsoft/vscode#48025 (comment)

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
Copy link

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
Copy link

jpike88 commented Apr 28, 2018

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
Copy link

shinriyo commented Sep 6, 2018

VSCode never supports?

Or, I want plugin or I'll develop myself

@octref
Copy link

octref commented Sep 6, 2018

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
Copy link

dwelle commented Nov 14, 2018

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

@mjbvz
Copy link
Contributor Author

mjbvz commented Nov 14, 2018

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

@dwelle
Copy link

dwelle commented Nov 14, 2018

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

{
    "compilerOptions": {
        "rootDir": ".",
        "module": "system",
        "resolveJsonModule": true
    },
    /* ... */
}

@dwelle
Copy link

dwelle commented Nov 16, 2018

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
Copy link

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

@ahaq0
Copy link

ahaq0 commented Nov 23, 2019

Currently, what's the best workaround?

@Lexicality
Copy link

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

The problems my team and I currently have are:

  • Importing a file that doesn't exist should be a lint error. This is technically possible with ESLint but the plugin is incredibly slow
  • The imports from the non-js file should be type checked (this is currently handled via ambient module declarations, but I thought it's worth mentioning)
  • Typing an import statement in VSCode should show all the files I've configured to be importable, eg css files, template files and so on
  • Pressing F12 on a non-js import should take me to that file in VSCode.

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
Copy link

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
Copy link

DMQ commented Mar 13, 2020

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

@Cpanmac
Copy link

Cpanmac commented Apr 17, 2020

When can we deal with this problem?@mjbvz

@mishannn
Copy link

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
Copy link

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
Copy link

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

It planned to support import resolution in a future release.

@dummdidumm
Copy link

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
Copy link

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
Copy link

chkpnt commented Sep 16, 2021

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
Copy link

@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
Copy link

chkpnt commented Feb 8, 2022

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
Copy link

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

@shankarThiyagaraajan
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Domain: JavaScript The issue relates to JavaScript specifically Domain: Symbol Navigation Relates to go-to-definition, find-all-references, highlighting/occurrences. Suggestion An idea for TypeScript VS Code Tracked There is a VS Code equivalent to this issue
Projects
None yet
Development

No branches or pull requests