[core] Add import extensions

[michaldudak]

Used fix-esm-import-path to add extensions to all our relative imports and exports in the Base UI package.

It is needed to implement #745 correctly.

[mui-bot]

Netlify deploy preview

https://deploy-preview-812--base-ui.netlify.app/

Generated by 🚫 dangerJS against aebc7ee

[Janpot]

I would advise against this. I believe it will make your files impossible to execute without a build step, e.g. with tsx. In core repo this is used by certain scripts (i18n I think). This restriction won't be there if you keep the imports bare and instead add the extension during transpilation.

[michaldudak]

I just checked that tsx works without any issues with .js extensions. Node with --experimental-strip-types doesn't work, but neither does it with extensionless imports (it has to have a .ts extension). Given that it's experimental, I wouldn't worry about it at this point.
Besides, the package code is not meant to be run directly.

This restriction won't be there if you keep the imports bare and instead add the extension during transpilation.

We can add extensions pretty easily to the build output, however it's not the case with type declaration files. Since they are produced with TypeScript, we can't run any transforms during their generation AFAIK. We could theoretically take the built definition files and add extensions to their imports as an additional build step, but I haven't found a tool that does this reliably (the one I mentioned in the PR description required some manual steps).

[Janpot]

Why do we need extensions in declaration files? don't they just follow typescript resolution? The only tool that cares about them is the typescript compiler and unlike ESM hosts, it knows how to resolve bare imports correctly.

[michaldudak]

There are issues related to this. I described them in #745. According to the TS team, a declaration file should be related to exactly one implementation file, so ESM output should have its own declarations. And, since it's ESM, it should have extensions.

[michaldudak]

For the record, I'm not a fan of importing output files in the source code, to say the least, and I'm glad that Typescript will support rewriting relative import paths soon. I am considering using the .ts(x) extensions once https://devblogs.microsoft.com/typescript/announcing-typescript-5-7-beta/#path-rewriting-for-relative-paths lands.

[Janpot]

And, since it's ESM, it should have extensions

I don't think that's how resolution works with declaration files.

in every module resolution mode, the compiler always looks for TypeScript and declaration files first, and if it finds one, it doesn’t continue looking for the corresponding JavaScript file. If it finds a TypeScript input file, it knows a JavaScript file will exist after compilation, and if it finds a declaration file, it knows a compilation (perhaps someone else’s) already happened and created a JavaScript file at the same time as the declaration file.

https://www.typescriptlang.org/docs/handbook/modules/theory.html#the-role-of-declaration-files

[michaldudak]

https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/FalseCJS.md#explanation

When a user writes an import, TypeScript needs to know whether the resolved module is ESM or CJS in order to provide accurate checking. It makes this determination based on the file extension and package.json "type" of the type declaration file it finds.

[Janpot]

exactly, if you generate your declaration files next to your esm files and both are under a folder with a package.json with type module, then all is fine, no? no need to change the imports in declaration files, as far as i understand.

[michaldudak]

If they are in a directory with type="module" and moduleResolution is set to Node16 or NodeNext, these files must conform to this resolution strategy's requirements as well (= they must have extensions).

[Janpot]

does that same restriction apply when you use the mjs extension instead? it has the added benefit of making life simpler for resolvers. they don't need to traverse up in the file tree looking for out of band esm configurations

[michaldudak]

Yes, it doesn't matter.
I created a small reproduction that shows the issue: https://github.com/michaldudak/ts-resolution-strategy

I authored the .d.mts files by hand and omitted the extension. I added both the .d.mts extension and type: module to the parent package.json and I'm still getting errors while building the app project.

[Janpot]

Ok, I see. An alternative could be to run tsc-alias with its resolveFullPaths setting on the generated declaration files. That seems to do the trick as well.

[michaldudak]

👍, yup, this seems to work at first glance (assuming the declaration files are identical when built for nodenext and esnext/bundler, but it seems to be the case). I can explore it further.

Having extensionless imports in source would still require us to use a transform after building. If we go with extensions, we could potentially build the project using tsc at some point, saving some CI time.

Is there an objective reason you're against having extensions in the source, or is it just your preference?

[Janpot]

Is there an objective reason you're against having extensions in the source, or is it just your preference?

Yes ofcourse, from the point of tsc it's clear. They don't want to add any transformations that alter javascript semantics. And I respect that, it's about scope management for them. But it also means they position themselves as a type checker primarily and a builder/bundler optionally. It looks like they rather delegate that task to tools that are (or will be) better suited and more performant. I'm thinking rust based tools that support features such as variable replacement, resolving import specifiers, code bundling, aliasing etc... And therefore I believe it makes more sense for us to rather work towards moving away fully from tsc for any other purpose than type checking. We can leverage bundlers to reduce the amount of modules we export and add other compile time optimizations (e.g. react compiler). My hope is that with upcoming typescript features such as isolated declarations we'll be able to move away from tsc for declaration generation as well. If the bundler handles both js compilation and declaration, it evidently will write correctly resolved import specifiers for the desired target.

As from typescript point of view it makes sense to me that they don't want to be much more than a type checker for now. But from a user point of view it doesn't. We want access to transpiler and bundler features, and we want to reference files without their extension, or if we must at least with an extension that makes sense (.ts). That seems rather evident to me, from a user point of view, how would it ever make sense to reference a typescript file using a .js extension?

In the meantime ofcourse we need a solution, I slightly lean towards avoiding .js extensions in all of our code bases if we can do it in a reasonable way, but not at any cost. Depends on where we envision our tooling to go, above is my opinion, but open for debate ofcourse.

[michaldudak]

They don't want to add any transformations that alter javascript semantics. And I respect that, it's about scope management for them

Not necessarily - they figured that rewriting extensions might cause issues and deliberately did not allow this:

There are several reasons for this, but the most obvious one is dynamic imports. If a developer writes the following, it’s not trivial to handle the path that import receives. In fact, it’s impossible to override the behavior of import within any dependencies.

function getPath() {
    if (Math.random() < 0.5) {
        return "./foo.ts";
   }
   else {
       return "./foo.js";
   }
}

let myImport = await import(getPath());

This is a problem that even a bundler cannot solve, so in some cases, the .js extension has to be used.

Now, this issue is very unlikely to affect us, as we don't use dynamic imports in library code.

---

Having the .ts extension in static imports (assuming using TS 5.7) would actually make code even more universal (re your point about making files impossible to execute without a build step), as it could be runnable by tsx and plain Node (with --experimental-transform-types).

---

I created a PR for outputting ESM+CJS without the need for extensions in import specifiers: #821. It seems to be working well, so if you don't mind, we can work on merging it and then continue discussing the import extensions separately.