Add `--allowNonJsExtensions`, a flag for allowing arbitrary extensions on import paths

[weswigham]

And, correspondingly, the .d.ext.ts files needed to describe the shape of whatever it is your importing.

Implements #50133

Option name, as always, open to 🚴🏚️.

[Thundercraft5]

A name suggestion: How about --resolveArbitraryExtensions or --allowArbitraryFileExtensions?

[weswigham]

Yeah, we only do the new fs check when the extension in the import isn't a TS or JS one.

[andrewbranch]

I’ve only just glanced at the implementation, but re: naming—in #51171 I introduce allowImportingTsExtensions, and I think I’ve managed to convince myself that its functionality really shouldn’t be combined with this flag you’ve added, because

• Importing .ts extensions requires noEmit or emitDeclarationOnly, whereas this flag does not.
• If you specifically indicate that you want to import .ts extensions, we can use that knowledge in path completions and auto-imports.

To that end, I would like to brainstorm flag names that don’t seem to imply .ts. I think “arbitrary” is a step in the right direction; maybe “unrecognized”? I haven’t thought of anything I really like yet, just wanted to throw that out and get thoughts.

[andrewbranch]

Also: don’t you need corresponding changes in moduleSpecifiers.ts and path completions? E.g., what happens in declaration emit when you have:

// foo.d.html.ts
export declare class CustomHtmlRepresentationThing {}
export declare const someHtml: CustomHtmlRepresentationThing;


// index.ts
import { someHtml } from "./foo.html";
export function getHtml() {
  return someHtml;
}

[weswigham]

@andrewbranch Done - I've also simplified things in the module name resolver quite a bit, in part thanks to your refactorings. As an added bonus, .ts file imports now resolve during module resolution, but we still issue retain an error on the import in the checker, for our usual "ts files won't resolve at runtime" reasons. Should make allowImportingTsExtensions pretty easy to implement~

[andrewbranch]

As an added bonus, .ts file imports now resolve during module resolution, but we still issue retain an error on the import in the checker, for our usual "ts files won't resolve at runtime" reasons. Should make allowImportingTsExtensions pretty easy to implement~

I’ve already done this too 😅 let the merge conflicts continue!

[weswigham]

@andrewbranch conflicts resolved, there's a minor change in the bundler resolution mode baseline; namely changing module resolution to consistently prefer loading the associated .ts source file, even when the import explicitly says .d.ts. There's also some weirdness with .tsx loading that we really haven't resolved (historically a .js file of the same name would get loaded at a higher priority than the .jsx file, even if you imported the .jsx directly - bunder mode kinda changed that with it's exact match branch, and I've formally encoded the change in behavior for tsx/ts while removing the bundler mode special case in module loading), though maybe it's not terribly important to resolve, since having a a.ts and a.tsx side-by-side is, as far as we're concerned, not really supported anyway.

[weswigham]

@andrewbranch done~

[weswigham]

It just means we're over-invalidating incremental builds a little bit, since we're tossing resolutions that we could probably reuse if the option value is changed. Honestly not too high impact, so we don't need to squeeze it into the beta if it'll delay things, but we can probably ship a fix in the rc no problem, since a fix is just changing the field name here to another one.

On Thu, Jan 26, 2023, 11:40 AM Jake Bailey @.***> wrote:

@.**** commented on this pull request.

In src/compiler/commandLineParser.ts https://github.com/microsoft/TypeScript/pull/51435#discussion_r1088267431 :

@@ -1206,6 +1206,14 @@ const commandOptionsWithoutBuild: CommandLineOption[] = [ description: Diagnostics.Enable_importing_json_files, defaultValueDescription: false, },

• {

•    name: "allowArbitraryExtensions",
  

•    type: "boolean",
  

•    affectsModuleResolution: true,
  

Is this something that should be in 5.0 beta? I just got to this comment in my email and noticed a PR wasn't sent for this (but I don't know how these two settings matter myself)

— Reply to this email directly, view it on GitHub https://github.com/microsoft/TypeScript/pull/51435#discussion_r1088267431, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAWMAMTG6YIP7AQ4B6R4WSTWULHMLANCNFSM6AAAAAARZXVPPM . You are receiving this because you modified the open/close state.Message ID: @.***>

[jakebailey]

I sent #52437 for it just to have it off of my mind 😅

[sheetalkamat]

Apart from incremental build, it also means that source files are not reused in LS if affectsModuleResolution is set to true. And that has perf impact so something good to have i guess.