Repository navigation
Fix checkJs behavior for .mjs and .cjs files next to declarations - #64527
Hardik K (hardikkaurani) wants to merge 1 commit into
Conversation
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
|
Looks like this PR has a chance to resolve #63523 too. 😀 |
|
Jimmy Leung (@hkleungai) Great catch! Yes, this PR should indeed fix the inconsistency reported in #63523. By adding the identical exemption to Ryan Cavanaugh (@RyanCavanaugh) Just a heads up! I've linked this PR to #64312 to ensure consistent behavior for .mjs/.d.mts\ and .cjs/.d.cts\ files, mirroring the existing .js/.d.ts\ legacy behavior. As an added benefit, this also patches the wildcard parsing order dependency noted in #63523, making the behavior deterministic regardless of the \include\ array order. Let me know if you have any questions or feedback! |
|
No tests? |
|
Jake Bailey (@jakebailey) Thanks for the feedback! I've added a regression test in |
|
...Why would we make the new extensions have an explicitly legacy behavior? |
|
If anything, we should remove the legacy lookup behavior from |
|
Wesley Wigham (@weswigham) I looked further into the existing extension-priority behavior and its history before making any changes. The Given that, extending the existing For #64312, the issue explicitly prioritizes consistency over which behavior is chosen. One consistent approach would therefore be to remove the legacy Since that would intentionally change existing |
One can imagine that, if projects are running so fast, if old js source files keep growing, then adding explicit declaration files would be (one of) the most non-interuptive way for adding some flavour of typechecking. Kinda like C-style codebase, with all the I am intentionally skipping the inline jsdoc approach in above. In terms of typegen, for some people, jsdoc is a bit harder to be done right, than having explicit typedef files. Not sure whether or not this coexisting pattern is becoming too uncommon in js world, but I would say in older versions of typescript, it is a possible way to do things :) |
|
Jimmy Leung (@hkleungai) That's a fair point. Authoring a The distinction here, though, is between the validity of that pattern and how the compiler handles those files when they are discovered together through wildcard inclusion. When TypeScript discovers both If a project needs to type-check the JavaScript implementation against its declarations, it can explicitly include both files via the That still leaves the compatibility question around the existing |
Imo, "switching" to Or I should say, this approach of digging deep into the distinction on Naively, in user perspective, I kinda hope I can put the all these sources as-is on |
|
Jimmy Leung (@hkleungai) I agree with that. I don't think the intended fix should require users to switch from I only mentioned explicit inclusion to distinguish intentional coexistence from wildcard discovery; I don't mean to suggest For this PR, I think the important question is how I'll leave the compatibility/design decision around the existing |
|
Wesley Wigham (@weswigham) i intentionally include a .js and .d.ts in all of my typed npm packages, and intend to continue to do so. That's the only way to write typed JS (without writing TS) that I'm aware of. |
|
Jordan Harband (@ljharb) Thanks for sharing that context! It's really helpful to see concrete examples of this pattern being actively used in the ecosystem. Wesley Wigham (@weswigham) Given Jordan's feedback that authoring a If so, extending this existing behavior to Would you be open to proceeding with the current approach in light of this? |
No.
Yeah, that's normal, and has no bearing on If you mean "I write my packages with manually authored I'd be happier to fix
We emit declaration files for jsdoc annotated js, which, as of TS7, has a much much closer correspondence to how we check and declaration emit TS files. That is the well-supported, non-footgun "how do I check my JS" path - not the weird |
|
Wesley Wigham (@weswigham) I've updated the implementation according to your feedback. We removed the legacy .js exception from the extension priority logic during wildcard resolution instead of extending it to modern extensions. This aligns legacy .js behavior with modern .mjs/.cjs behavior, creating a consistent and deterministic include resolution that prevents the configuration footguns. I've also updated the regression tests to assert that .js, .mjs, and .cjs implementations are consistently skipped when higher priority .d.ts, .d.mts, and .d.cts files are present. |
|
I'm confused what's a footgun about the .js behavior? The footgun I see is that adding one file can cause another to silently become unchecked. I don't see why how common it is is relevant at all? |
|
Jordan Harband (@ljharb) Wesley Wigham (@weswigham) I think the distinction here is between the I don't mean that having a With the latest change, we're not trying to prevent the This keeps the change focused on making wildcard file selection consistent and deterministic across the extension families, rather than making a statement about whether the If this matches the intended behavior for |
|
ok but that's worse - because it indeed is silently skipping a file solely because i created another one, without any config changes. That should never be the case. |
|
Jordan Harband (@ljharb) Thanks, that clarifies the compatibility concern. I understand that removing the legacy Wesley Wigham (@weswigham), I don't want to make another assumption here. Could you clarify the intended compatibility semantics for wildcard resolution? Jordan's example shows that the current change alters the behavior of an existing |
|
We're either doing nothing or removing the legacy dual |
|
Wesley Wigham (@weswigham) Thanks, that clarifies the intended direction. I’ve kept the change focused on removing the legacy .js/.d.ts\ wildcard behavior rather than extending it to .mjs\ and .cjs. I’ve also kept the regression coverage aligned across the three extension pairs. |
|
Wesley Wigham (@weswigham) can you help me understand why the It seems perfectly correct to me that any file with a corresponding dts file (js/dts, mjs/mts, cjs/cts) would typecheck both the file itself, and for all external consumers, only use the types of the dts file. Why would you ever want a file to be implicitly not typechecked? |
…crosoft#64312) Drop legacy .js/.d.ts wildcard exception from extension priority to align behavior across .js, .mjs, and .cjs. Fixes microsoft#64312
466fc94 to
06daa79
Compare
Following maintainer guidance, this PR removes the legacy
.js/.d.tswildcardincludeexception rather than extending that historical behavior to.mjsand.cjs.Summary of Changes:
includenow uniformly prioritizes declarations across all module formats (.d.ts>.js,.d.mts>.mjs,.d.cts>.cjs). Lower-priority implementation files are omitted when companion declarations are present..js+.d.ts,.mjs+.d.mts,.cjs+.d.cts) undercheckJs.include.Note: Users who intentionally author companion implementation and declaration files side-by-side can continue to load them explicitly via the
filesarray or triple-slash reference directives.Fixes #64312