Skip to content

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

Merged
pi0 merged 1 commit intounjs:mainfrom
webpro:fix/resolve-js-to-ts
Nov 1, 2024
Merged

fix: try to resolve .ts files with .js extension from js files#337
pi0 merged 1 commit intounjs:mainfrom
webpro:fix/resolve-js-to-ts

Conversation

@webpro
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
@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