-
Notifications
You must be signed in to change notification settings - Fork 10
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
Support ts-node
by checking .ts
source files
#47
base: master
Are you sure you want to change the base?
Conversation
Note the build failures already existed on |
Hi @marshall007 , thanks for this PR. Yes the CI failure was caused by an accidentally committed file by me. Along with other feature requests, I think there would be a major refactoring for clime in the near future. And this is PR is has some intersections with #42 , which addresses this issue in a configurable approach. I am actually okay with both, so it would be nice if you may update the correspondent part of As for tests, you can check out https://github.com/vilic/clime/blob/master/src/test/baseman/cli-test.ts#L45 I think maybe we can simply duplicate all |
@vilic I just pushed up another commit addressing the remaining cases in |
@marshall007 You might be right. We need some efforts on baseman implementation though. I will make necessary changes to baseman and test case generating here, and once done will merge your PR (if it passes). Thanks! |
@vilic no problem, just let me know if you need anything else! Cheers! |
src/internal-util/fs.ts
Outdated
@@ -11,6 +12,28 @@ export async function existsFile(path: string): Promise<boolean> { | |||
return !!stats && stats.isFile(); | |||
} | |||
|
|||
export async function existsSourceFile(path: string, filename: string = 'default'): Promise<string | undefined> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not simply use require.resolve
to resolve a module with any supported file extension?
Should fix #41, but I haven't been able to test yet.
I still have a few more cases to update like in
help.ts#L108-L117
, but I wanted to run this PR by you before continuing (mostly because I'm unsure how we should go about adding tests for this).For what it's worth, I also don't think this is a good long-term solution. I would really like to see #43 implemented, but this seemed like the easier path forward for now to get my feet wet.