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 explicit extension support when using Typescript and ESM #1073

Merged
merged 1 commit into from
Jul 26, 2024

Conversation

Marsup
Copy link
Contributor

@Marsup Marsup commented Jul 25, 2024

The convention on Typescript when using ESM is to require .js files while actually expecting .ts files to be imported, this fails with current lab as obviously the .js file doesn't exist and the require.extentions hook doesn't even get a chance to run.

I inspired myself from ts-node for this (especially https://github.com/TypeStrong/ts-node/blob/main/src/cjs-resolve-hooks.ts), but I don't think I need everything as this is just enough to make it work. This is very specifically tailored for the scenario I'm seeing, I'm not sure that it covers everything out there.

Copy link
Contributor

@damusix damusix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

noice

@Marsup Marsup added the feature New functionality or improvement label Jul 26, 2024
@Marsup Marsup added this to the 25.3.0 milestone Jul 26, 2024
@Marsup Marsup merged commit 78ec46c into master Jul 26, 2024
10 of 15 checks passed
@Marsup Marsup deleted the feat/typescript-esm branch July 26, 2024 06:26
Copy link
Contributor

@kanongil kanongil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to guard against adding the hook multiple times (or for sure only call it once).

Also not a fan of the embedded require(), which goes against the hapi style guide.

@Marsup
Copy link
Contributor Author

Marsup commented Jul 28, 2024

This needs to guard against adding the hook multiple times (or for sure only call it once).

This is the CLI execution, it's only ever called once AFAIK, at least in normal conditions.

Also not a fan of the embedded require(), which goes against the hapi style guide.

This file is plastered with requires that are not top-level, I don't mind making it global but I also don't see the point if it's never used. Is it important for you?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants