-
Notifications
You must be signed in to change notification settings - Fork 22
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
Resolving paths using path aliases #127
Comments
Hey @kevin-st, thanks for reporting the issue and providing that very detailed repro.
It's not the same issue so you were right to open it, the #124 is about supporting Node.js path aliases while the issue you're encountering is only specific to TypeScript path aliases.
This is exactly how it's supposed to work (and how actually it works for non-Windows environments). From the very quick investigation I did the issue only seems related to Windows environment where paths are handled in a different way. On MacOS, I was not able to reproduce the problems you mentioned on your repo, could you please confirm me that it's the same for you if you have a Mac near by? I'll create a StackBlitz sandbox tomorrow to provide you an example to be sure that this is only a Windows problem. The issue seems to come from the resolution step because paths aliases registered are weirdly mapped to anti slashes hence the resolver does not consider So essentially I think that this is not related to the symbols I'll investigate in more details tomorrow, I'll keep you updated. |
We have checked it on a Mac and it's working there as expected, so it seems to be a Windows issue indeed. I also tried to run it on WSL (Windows Subsystem for Linux), so I would expect it to run the same as on Mac, but it seems as if something's wrong there as well and that could be a separate issue. Regarding this issue, we've been trying to look into it, but we haven't found the cause yet. However, I can share the following:
We see that the code is coming in the On Windows, we see that the Following the
and That value is not set in the map, so we reach the while-loop in the function. This while-loop is never executed though, as while (!isNotBasePathSegment(pathSegmentToMatch) && pathDepthAttempts < 10) Note: Personally, I think the After making this change, the function returns true. However, something else seems to go wrong if we do it like this and from there on we currently have no clue yet what the issue exactly is. Maybe this gives some insight. |
Thanks for the detailed analysis, it confirms the hypothesis that I had even though I didn't have time to deep dive, problem is I need to prepare few meetups, one this week and the other one next week, so I'm not really sure if I'll have time to fix the issue until middle of next week. You have already spent a lot of time deeply investigating through that issue and I appreciate it, if you ever find a fix that solves the issue while keeping all the tests to green (+ the one that will be added to cover the current case) I'll be able to merge/publish the fix in the meantime. Otherwise I'll take the lead fix the issue asap, I have a Windows computer that I can use to troubleshoot the issue, it will be easier for me to fix it, for now it's only speculation on my side. I also take notes about the naming suggestions there :) Feel free to keep me updated if you guys ever find other interesting stuff to share. Thanks |
Hello @kevin-st, Great job investigating! As previously said I didn't have time myself to do it and I won't have time before tomorrow, but I highly appreciate your analysis. To be honest I'm kind of confused that cross platform path issue was not covered yet, I might have mis-used the API in some kind of way, replacing double slashes might be one solution or maybe avoid relying on The fact that you don't have an link (arrow) where it should means that there is a mismatch between the graph representation and the module resolution so the relationship won't be established. The node's path needs to be exactly the same as when it's resolved and when it's put in the graph (both for the node and the directed link), otherwise it won't show up. Consequently resolving correctly the alias is one thing, but then we should map and add the real path (existing in the alias map) within the graph.
Unit tests are there for that normally, if that's not enough yet it means that I might need to add Windows machine on the matrix to be sure that they are all running properly on Windows as well. The outcome when running Do you have that repro nearby that I could locally pull for the latest example? I'll start setting up my Windows machine and help you out resolving the thing. |
@kevin-st as shown in this workflow Windows platform has some unit tests failing (not even Starting tomorrow I'm catching up this subject and other things at the same time to make |
Hello @kevin-st, Just opened a pull request (#128) that will, I hope, fix all windows-related issues, including the one you described there. Here is an overview of the graph I got with skott's version from the pull request based on the repro you provided me: It now seems to be behaving as expected, correctly following TypeScript's path aliases. I'll let you know when the release is published so that you can test on your own. |
Hello @kevin-st, Just published Thanks again for submitting the issue and providing such detailed information. |
Hi @antoine-coulon, I'm so sorry for my late response; with the holidays kicking in I completely forgot about the issue. I just checked the update and everything seems to be working as expected. Thanks for taking the time to look at this! |
Hello @kevin-st, no worries, no news is good news with open-source. Thanks for letting me know I'm glad it works now, don't hesitate to re-open issues if you still encounter some problems (related to this or not) :) |
Summary
When using path aliases which are prefixed (e.g. using "@", "#" or "~"), paths can't be resolved correctly. Unless the prefix is also present in the name of the folder on the filesystem, files can't be found.
I'm not certain if this is really a bug or if it's working as expected, but it can't hurt to share the issue.
Note: I've seen #124 which seems to be kind of the same thing, but I'm not certain. I haven't heard yet of "import maps" so in case I'm talking about the same issue here, feel free to mark it as duplicate.
Reproduction steps
You can find a simplified example of the problem at the following repository, which includes a README on how to setup the repo:
https://github.com/kevin-st/skott-example
I also added a logs folder, which contains the output of the commands.
Details
We're using prefixed aliases (using an "@"-symbol) in a project I'm working on. The project is setup with Webpack and TypeScript and follows a similar configuration as found in the repository linked up here.
When using these "@"-symbols in the path aliases the import statements are written like this:
and the path alias is configured as follows:
and the actual folder on the file-system is called
ar
.If we look at the logs, we see that it's trying to resolve the path as follows:
which can be basically seen as:
tsConfig.baseUrl + import statement
.However, earlier on in the logs, we can see that it's registering the path alias:
I'm under the impression that the wrong information is being used to resolve the paths. I'd think that it should be able to use the information from the registration (being
@ar
which is mapped topackages\ar
) and replace that in the import statement when trying to resolve it.e.g. if the import statement is written like:
then I'd think it would be possible to replace the
@ar
withpackages\ar
, making the final import statementpackages\ar/components/ARComponent
. (This is me thinking the easy way 🤷♂️)Sidenote: The easy solution on my side would be to get rid of the prefixes in our path aliases, then the issue is fixed. It still leaves me wondering though if this is working as expected 😅
Standard questions
Please answer these questions to help us investigate your issue more quickly:
skott
installed version?node -v
)?The text was updated successfully, but these errors were encountered: