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

fix: try to resolve .ts files with .js extension from js files #337

Merged
merged 1 commit into from
Nov 1, 2024

Conversation

webpro
Copy link
Contributor

@webpro webpro commented Nov 1, 2024

Resolves #334

Interestingly, with this change tests still pass and the use case is fixed. The change covers a regular use case I think: id's ending with JS resolve to a file with TS extension. BUT I'm probably missing something here, so please verify.

Btw, what's your dev mode to set breakpoints in source code directly (without build, register etc), is it possible? I often do something like tsx --inspect src/cli.ts but didn't succeed to create something like this.

// Try resolving .ts files with .js extension
if (
TS_EXT_RE.test(ctx.filename) ||
TS_EXT_RE.test(ctx.parentModule?.filename || "")
Copy link
Member

Choose a reason for hiding this comment

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

While tests might pass (and I immediately cannot recall why we needed this check) it might be a safer option to add new checks in addition to the current ones.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah just to make you aware of the situation. Pushed a minimal change/fix now.

@pi0 pi0 changed the title Try to resolve .ts file based on .js ext of id fix: try to resolve .ts files with .js extension from .js files Nov 1, 2024
@pi0 pi0 changed the title fix: try to resolve .ts files with .js extension from .js files fix: try to resolve .ts files with .js extension from js files Nov 1, 2024
@webpro webpro force-pushed the fix/resolve-js-to-ts branch from d8e814e to 7aba248 Compare November 1, 2024 10:45
@webpro webpro marked this pull request as ready for review November 1, 2024 10:47
Copy link
Member

@pi0 pi0 left a comment

Choose a reason for hiding this comment

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

Thnx!

@pi0 pi0 merged commit f57e301 into unjs:main Nov 1, 2024
7 checks passed
@webpro webpro deleted the fix/resolve-js-to-ts branch November 1, 2024 10:49
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.

Importing ts file with .js specifier from .mjs in another workspace fails
2 participants