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

feat: add fallback option to avoid meaningless result. #66

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

emosheeep
Copy link

@emosheeep emosheeep commented Jan 12, 2024

#65

@emosheeep emosheeep force-pushed the feat/remove-meaningless-result branch from 3e42d9d to be179ac Compare January 12, 2024 15:51
@privatenumber
Copy link
Owner

Why would we need to accept a fallback if TypeScript doesn't accept a fallback?

@emosheeep
Copy link
Author

emosheeep commented Jan 12, 2024

I was not sure if this is an intending behavior so I made it a feature not a bug fix. we can return an empty array when matched value is null on condition that you're sure the behavior to join baseUrl and a mismatched alias is really meaningless.🤔

@emosheeep
Copy link
Author

how do you think about it ?🌚

@privatenumber
Copy link
Owner

I haven't had time to take a look yet, but very high chance I won't be accepting any options TypeScript doesn't accept.

The point of this project is feature parity so it should do exactly what TypeScript does.

@emosheeep
Copy link
Author

emmm does it mean ts will join baseUrl and alias directly even if the path is meaningless ?Sorry I don't aware of this at that moment,if it is, just keep it as it was.

@privatenumber
Copy link
Owner

Again, I haven't looked into this at all — haven't even understood what problem it's addressing.

But for the tests, it runs TypeScript to get it's correct behavior via getTscResolution. Have you tried comparing results to that?

@emosheeep
Copy link
Author

It seems that getTscResolution function calculated these:
image
It did join them together...what confused me a lot...but as what you said, if this project aims to be a feature, maybe it can extend this behavior and remove the meaningless part.

For a better user experiences, it’ll be convenient if the resolved path is reliable, because it’s easy to judge a null value but hard to a wrong path, which may need to call the filesystem and manually try possible paths, it's not efficient on performance usually.

On the end, it’s your project and depends on you, what I thought about is just a reference, sorry to bother you, I’m not in emergency, you have enough time.

Have a good day~

@privatenumber privatenumber changed the base branch from develop to master May 3, 2024 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants