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

import doesn't resolve to correct directory #3

Closed
Aranir opened this issue Apr 22, 2016 · 11 comments
Closed

import doesn't resolve to correct directory #3

Aranir opened this issue Apr 22, 2016 · 11 comments
Assignees
Labels

Comments

@Aranir
Copy link
Contributor

Aranir commented Apr 22, 2016

The import statements always refer to the current directory:

import {Test} from './Test';

even if the file is located in a different directory.

Is there a way for the autoimport to work with the correct relative path?

@cybertim
Copy link
Owner

It should work with different directories relevant to the current file within the same workspace, for me, at least :-)
import {Test} from '../../Test' for instance if it is in a 'lower' directory or import {Test} from './test/Test' if it is higher up.
Do you have an example structure of the workspace in which it is not working?

@Aranir
Copy link
Contributor Author

Aranir commented Apr 22, 2016

I created quickly a repo where it fails:
link to github repo

As can be seen if the import command is used it doesn't resolve the directory in a correct way.

This might have something to do with me working on a Mac.

Let me know if you need further input

@cybertim
Copy link
Owner

If I import in the app.ts the non-wildcard imports resolve like:

import {router} from './routes/index';
import {user} from './routes/users';
import {Simple} from './commonsimplenesteddeep/Simple';

Basically the first two are correct (one deep is working fine) but deeper has a bug which is simple to fix (will update the extension probably today).

But based on your first comment, importing should not resolve paths at all and always use ./?

@Aranir
Copy link
Contributor Author

Aranir commented Apr 22, 2016

Yes, I don't know if the two problems are linked, my real project is a bit more complex and uses webpack.

Not sure if that might be a cause of the issue.

@cybertim
Copy link
Owner

Is it possible to make or alter a project so it has the same issue? So I can fix all the problems at once :-)

Also, when you add an 'un'-imported but exported item you get intellisense completion.
If you do this for an item that has an incomplete import line you can see the path it lists.
If this path is higher or lower it should create the import-line accordingly. If it is the same, the path is indeed ./, this is one way to see if it is related to the project/webpack.

@cybertim
Copy link
Owner

Tonight for some reason I suddenly realised that there is a bug in the way I process paths. If the files are in the same path-depth the extension will think they are in the same directory.
This is probably what is happening:
/path/to/first/file and /path/to/second/file
should resolve toimport {Test} from '../second/file' but this will resolve to './file'

I'm going to re-implement the 'path' system in a whole to solve this.

@cybertim cybertim added the bug label Apr 23, 2016
@cybertim cybertim self-assigned this Apr 23, 2016
@Aranir
Copy link
Contributor Author

Aranir commented Apr 23, 2016

I nearly have finished working on prototype where I used the path library of node for the path resolution.

Would it be ok if I push it on a separate branch and you can inspect it?

@cybertim
Copy link
Owner

Nice, would ike to take a look at it, I was also working on a (custom) different approach, since I was not sure if it's possible to include node libraries other than the vscode library.

BTW you can just push it to the main, since the real release is done through the vscode marketplace.

@Aranir
Copy link
Contributor Author

Aranir commented Apr 23, 2016

I couldn't push to the main (access rights), but I forked your repo and pushed on the fork.

Please have a look and let me know what you think.

@cybertim
Copy link
Owner

Thanks for the update. I merged it and fixed an issue when matching import lines, now it seems to work on all path structures I can think of.

I thought the path lib was a third party thing, but it's integrated in node, I should have done this from the beginning :-) this will prevent lots of other weird things in the future.

I will push the new version to the market later this day.

@Aranir
Copy link
Contributor Author

Aranir commented Apr 23, 2016

Great, thank you for accepting the pull request.

So this should be resolved now :)

@Aranir Aranir closed this as completed Apr 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants