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

[Bug] Paths needlessly go back to root on macOS #167

Closed
nathtumlin-tanium opened this issue Nov 14, 2022 · 6 comments · Fixed by #171
Closed

[Bug] Paths needlessly go back to root on macOS #167

nathtumlin-tanium opened this issue Nov 14, 2022 · 6 comments · Fixed by #171
Labels
Bug Something isn't working Help Wanted Extra attention is needed

Comments

@nathtumlin-tanium
Copy link

macOS has case-insensitive filenames and in resolve-module-name.ts srcFileOutputDir has a path starting with /Users/ and moduleFileOutputDir has a path starting with /users/ code link. This means that the path.relative call ends up ..ing all the way back up to / and creating a path like require('../../../../../users/nath/foo/src/bar').

I did some digging and it looks like in the SourceFile objects returned from the compiler API's getSourceFiles the fileName property has an uppercase Users and is constructed by concatting things with process.cwd() and things like path and resolvedPath have the lowercase users and I'm not sure where those come from.

Any thoughts on how to fix this either through code changes or a config change? Right now I'm just modifying the path.relative call to add some .toLowerCase()'s, but that won't work in general.

@nonara
Copy link
Collaborator

nonara commented Nov 30, 2022

Hello. That is an interesting problem.

I think the best route would be to do some debugging in this function

function getPathDetail(moduleName: string, resolvedModule: ResolvedModuleFull) {

Or the one which calls it to determine where and why we got a lowercase version.

Looks like there may also be some promise here: fs.realpathSync.native (https://stackoverflow.com/questions/33086985/how-to-obtain-case-exact-path-of-a-file-in-node-js-on-windows)

ATM, I don't have the bandwidth to investigate it deeper, but if you'd like to help track down where the issue is, I'd be happy to help get a fix through.

@nonara nonara added Bug Something isn't working Help Wanted Extra attention is needed labels Nov 30, 2022
@nathtumlin-tanium
Copy link
Author

I have a patch we're using locally that uses fs.realpathSync.native, I'll see if I can put it up here

@nonara
Copy link
Collaborator

nonara commented Dec 5, 2022

@nathtumlin-tanium Ok, great. If you can tell me where you patched it, I can push a fixed version.

@nathtumlin-tanium
Copy link
Author

I replaced the calls to path.relative in resolve-module-name.ts with a helper function that updates the from and to parameters with fs.realpathSync.native(from) and fs.realpathSync.native(to). Actually I update the values within a try/catch so that if the path doesn't exist on disk it doesn't error and just uses what it has.

Sorry, my company is a little strange about contributing code externally but hopefully that's enough to get it, it should be like a < 20 line diff.

bradennapier added a commit to bradennapier/typescript-transform-paths that referenced this issue Dec 15, 2022
fixes so all paths dont get rewritten to `../../../../../../../users/...` on osx 

fixes LeDDGroup#167
@bradennapier
Copy link
Contributor

@nathtumlin-tanium can you confirm the PR #171 is inline with your expectations? It works perfectly for my system but just in case I may have missed an edge case?

@nathtumlin-tanium
Copy link
Author

Yep, that's essentially the same as what we're doing. Thanks!

nonara pushed a commit to bradennapier/typescript-transform-paths that referenced this issue Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Help Wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants