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

"module": "node16" should support extension rewriting #49083

Closed
arendjr opened this issue May 12, 2022 · 99 comments
Closed

"module": "node16" should support extension rewriting #49083

arendjr opened this issue May 12, 2022 · 99 comments
Labels
Duplicate Out of Scope Suggestion

Comments

@arendjr
Copy link

arendjr commented May 12, 2022

Bug Report

TypeScript 4.7 RC introduces the "module": "node16" setting. When used, this requires users to use the .js extension in import paths. I strongly believe this to be a mistake and TypeScript should instead allow users to use the .ts/.tsx extension of the source file. It would then be the TS compiler’s responsibility to rewrite this to the output file as necessary.

I have several reasons for requesting this change:

  • The file with the .js extension most likely does not exist on disk. It may exist on disk if a build has been created and the output is placed right alongside the sources (not a common configuration), but otherwise the file is simply not there or in a different place than one might expect. This is confusing to users, because they need to manually consider how output files are created.
  • Using the .js extension makes the source code less portable for full-stack projects. If a user wants to use the same sources for a frontend project that uses a bundler, they will be out of luck. A bundler would be able to understand a reference to the .ts file (because that one actually exists on disk!), but would struggle with references to files that do not exist for its configuration.
  • Using the .js extension makes the source incompatible with Deno projects, as they use the .ts/.tsx extensions, because those are the ones that actually exist.
  • Using the .js extension is inconsistent with type import. Does import type { MyType } from “./myFile.js” even work? It would not make sense because the JS file contains no types. But if it doesn’t work, does that mean I still have to use the .ts extensions only for the types? That would be highly annoying.

🔎 Search Terms

extension rewriting esm module

🕗 Version & Regression Information

TypeScript 4.7. RC1

  • I was unable to test this on prior versions because the feature is new to version 4.7.

🙁 Actual behavior

I am forced to write import { myFunction, type MyType } from “./myFile.js” despite “./myFile.js” not being a valid path in my project.

🙂 Expected behavior

I should be able to write import { myFunction, type MyType } from “./myFile.ts” because “./myFile.ts” is the actual file I am referring to. Upon compilation I would expect TypeScript to rewrite the path so the output works correctly too.

@RyanCavanaugh RyanCavanaugh added Suggestion Out of Scope labels May 12, 2022
@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented May 12, 2022

We don't rewrite target-legal JavaScript code to be different JavaScript code from what it started with, no matter the configuration.

See also #16577 (comment)

@arendjr
Copy link
Author

arendjr commented May 12, 2022

I’m sorry, but I cannot understand why you would label this issue as Out of scope or why you would dismiss this based on a comment from almost one and a half year ago.

You argue “we don’t rewrite target-legal JavaScript code”. But this argument does not apply. If someone wrote import { myFunction, type MyType } from “./myFile.ts”, which I believe to be a very reasonable expectation from a user’s point of view, and you would emit that with the “.ts” extension intact, the resulting code would be invalid because that target doesn’t exist. You are building the compiler, you know that target doesn’t exist. So you wouldn’t be rewriting valid code, you would be rewriting code you know would be invalid if you didn’t rewrite it. I hope you’re not arguing it’s better to let the user shoot themselves in the foot.

Furthermore, the comment you linked depends on the arguments presented in yet another comment: #16577 (comment)

If you read the arguments there, this is clearly a different situation than what we are facing today. My issue is with the change in the new module resolution for ESM introduced in TypeScript 4.7. This change forces users to specify the .js extension. Compare this to the “facts” presented in the comment I linked, which clearly apply to a situation in which the user is free to choose whether they want to include the extension or not. This is not the situation as it is with the ESM resolution in TypeScript 4.7.

Back then, users could improve compatibility by adding the extension, but the way they are now forced to use it compromises compatibility as I attempted to explain, but none of which you addressed, and none of which any of the linked comments address.

As such I would kindly ask you to reconsider the implications of the new module resolution, including the impact on developers and the wider ecosystem.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented May 12, 2022

I don't think there's anything new to discuss here; the strong line we've drawn in the sand is that we do not rewrite import paths, ever. Our design principle, consistently, is that you write the import path that you want to appear in your JS file, and configure TS to tell it how to turn that path into a build-time path. Doing it differently is out of scope for our project.

This is a hard constraint per design goal number 7, "Preserve runtime behavior of all JavaScript code.". import { foo } from "./x" is JavaScript code. Granted import { foo, type bar } ... is not, but I think it's clearly way too confusing to have one runtime path appear if you put the type keyword on a single named import and a different path if you don't.

Nothing about node16 resolution changes this. Our principle even would be the same if some new module resolution system appeared where you wrote the sha256 hash of the file you wanted -- we would have you write import foo from "ba932c1fd3..." and then have a system to map that to foo.ts, not have you wrote import foo from "foo.ts" and then emit some JS other than what you wrote.

If you have some environment where you sometimes need to have JS that says import foo from "blah.js" and sometimes need to have JS that says import foo from "blah", that is also out of scope for our project, the same as if you needed the same input line of TypeScript to sometimes emit 3 * 4 and sometimes emit 3 + 4. There are tools to handle these problems and it's out of scope for us to re-implement solutions to those problems.

There are surely difficulties in navigating the module ecosystem, no one can deny that. What I'm saying is, rewriting import paths is not a solution we consider to be in-scope. We're always interesting in hearing ideas that can simplify the module resolution process and improve the user experience as long as it fits within our design criteria of not rewriting import paths.

@arendjr
Copy link
Author

arendjr commented May 13, 2022

Thanks for your reply. I understand I’m getting at some pretty fundamental issues here, so I try to navigate this as delicately as I can.

First, the new thing I mentioned is the node16 module resolution mechanism that comes with TypeScript 4.7. Unlike previous module resolutions supported by Typescript, this forces users to use the .js extension in their paths. While this may be a consequence of decisions that were made earlier, this does exacerbate the issues that result from those decisions. If nothing else, I believe that may present a valuable moment to reflect on those decisions to see if they still achieve the goals you set out to achieve.

I also believe the timing is important here. So far, and as far as I’m aware, Typescript users have mostly avoided including extensions in their paths. And for good reason: Including them makes their code less compatible with existing bundlers used in the ecosystem. One notable exception here are Deno users, who include the .ts extension, in a resolution mechanism that follows the ESM resolution in spirit.

If TypeScript 4.7 is released with the node16 setting as it’s implemented today, I’m afraid this may introduce further schism between the various user groups. This is unfortunate, because node16 is presented as the way to do ESM with TypeScript, but for users to use it they will be forced to move away from their current practices in a manner that is incompatible with their current tools. It will become harder for a single codebase to support Node.js and bundlers, while there is no technical reason for this to be so.

So while there’s no technical reason for this outcome you admit to be unfortunate, you do present one based on principle: You say you “do not rewrite import paths, ever” and back this up with the design goal that says to "Preserve runtime behavior of all JavaScript code."

So let’s reflect on this. Because if these principles/design goals will inevitably lead us to face such an unfortunate outcome, is it possible the design goal itself is in need for reconsideration? I would not necessarily go that far, but I do think your proverbial line in the sand to “not rewrite import paths, ever” is an unnecessarily strict interpretation of the design goal.

Let’s revisit my original request, and let’s simplify it to focus on the core issue:

import { myFunction } from ./myFile.ts”

Is this valid JavaScript code? It’s certainly syntactically valid. But if “./myFile.ts” does not exist or contains syntax that is not valid JS, it’s certainly not functional JavaScript code. Does it make sense to wish to preserve behavior of non-functional code?

But let’s take it a step further: How is this code supposed to behave? Again I will refer to Deno, because it defines actual runtime behavior for this that is inline with the ESM specification in spirit. But here’s the rub: Deno provides a TypeScript runtime, not merely a JavaScript one. So this code is valid TypeScript code, even if it is not functional JavaScript code.

Let’s look at that design goal one more time: "Preserve runtime behavior of all JavaScript code."

The behavior is clear. Just run it in Deno and see. But interestingly, it is the TypeScript code emit that breaks this behavior! Because after the code emit, the file it referred to is no longer there.

I know you will defend this by saying the path is intended to be an output path, but how does that follow from your design goal to not break behavior? If anything, this intention feels more like a post hoc rationalization to defend the “do not rewrite import paths, ever” position than it seems a useful part of the design. It’s certainly not a design goal.

Now, I realize there are no easy answers here. But from my perspective, the design goal to "Preserve runtime behavior of all JavaScript code" would be served by rewriting import paths if the behavior would otherwise be broken by your own emit process.

The statement import { myFunction } from “./myFile.ts” is valid TypeScript (even functional JavaScript under very strict circumstances) whose behavior would be broken if you do not rewrite the path during its emit phase.

While it is your prerogative to define the scope of your project, in my opinion it is a stretch to claim an issue introduced by your own process (it’s inevitable that paths break when you write files to a new location with new names) as out of scope.

Finally, I would like to come back to the hypothetical example you raised:

Our principle even would be the same if some new module resolution system appeared where you wrote the sha256 hash of the file you wanted -- we would have you write import foo from "ba932c1fd3..." and then have a system to map that to foo.ts, not have you wrote import foo from "foo.ts" and then emit some JS other than what you wrote.

What I find interesting here is that you say you’re willing to build a system to do the mapping, but you’re not willing to help the user writing the code. It seems you hold stronger to the notion of not rewriting import paths than to preservation of behavior or the notion of creating value for your users. We both have the same design goal in mind (to maintain the behavior of the code), but to me this shows your “do not rewrite import paths, ever” position is untenable and it feels not in line with what I believe would be the best interests of the project.

I hope I have not offended with this post. I raised this issue because I believe it would be harmful to the TypeScript community if segments of the community have to use incompatible module resolution schemes, which the node16 resolution as it stands further forces users towards. Instead, I hope that through a relatively simple change we can make people’s code more easily interoperable. Even if that change means a more fundamental reinterpretation of the original design goals.

@arendjr
Copy link
Author

arendjr commented May 21, 2022

As I fear this conversation might otherwise go entirely stale, I would like to offer one more argument as to why I think the “do not rewrite import paths, ever” position is not just undesirable, but actually runs counter to your own design goals.

Let’s look at another variation of the example we discussed so far:

import { myFunction, MyType } from ./myFile.ts”

Let’s assume MyType is not a class, but a TS type that gets erased during emit. What would the output be? It would become:

import { myFunction } from ./myFile.ts”

That’s interesting, isn’t it? The original snippet was 100% syntactically correct JavaScript. And yet it was rewritten as part of the emit process.

Of course rewriting it was the correct thing to do. Without rewriting, the emit process would have yielded output the JavaScript runtime would have trouble resolving. So we must rewrite JavaScript in order to preserve behavior.

Please explain to me how the import path is different. Sticking to “do not rewrite import paths, ever” is leading to output the JavaScript runtime will have trouble resolving — the exact same behavioral issues that you are willing to solve elsewhere. Issues you must solve in order to adhere to your own design goal of preserving behavior.

Of course you could argue that MyType refers to a TypeScript type, so you actually rewrote TypeScript rather than JavaScript. But in that case, please explain to me how the import path is different. “./myFile.ts” refers to a TypeScript file, so you would rewrite TypeScript rather than JavaScript.

Rewriting paths to TypeScript files in order to preserve behavior is compatible with your original design goal. To “not rewrite import paths, ever” is not.

@mlandalv
Copy link

mlandalv commented May 22, 2022

It feels like many of these problems would be solved if TS enforced the use of file extensions in import/export paths. Not .js but the actual extension, eg .ts or .tsx.

Then it's up to the transpilation process to output code compliant with the specified module and target. If that means changing .ts to .js then so be it.

(What if tsc eventually gets support for a --out-file-extension=.mjs flag. Then those .js in the paths won't work anymore.)

@s123121
Copy link

s123121 commented May 25, 2022

Technical aside, this issue make the DX worse for everyone. It doesn't make any sense for relative import to refer to a non-existent .js file. On the top of my hat, to circumvent this problem, the developer need to:

  • Change eslint setting for missing import
  • Some tools need to reconfigured to resolve non-existent resource (like Deno, ts-loader, ...)
  • vscode can navigate between files just fine but what about other text editor

@neil-morrison44
Copy link

neil-morrison44 commented May 25, 2022

I think some of the issue stems from this

is that you write the import path that you want to appear in your JS file, and configure TS to tell it how to turn that path into a build-time path.

as a user I see the typescript compiler taking valid TS code from the “src” realm and converting it to valid JS code in the “build” realm.

When I first heard about the “.js” extension it broke what I understood to be the separation between “src” & “build” - I wondered if I’d need to supply the full path to the output .js in the source file, or otherwise adjust it based on my knowledge of where the .js file would end up

@calebpitan
Copy link

calebpitan commented May 25, 2022

If only there was a way to put this to vote!

@RyanCavanaugh’s argument, which I believe represents that of the TypeScript team, doesn’t seem sustainable on the long run. If a community of other engineers says this is what they think is best, then it better be given a strong consideration, rather than being labeled “out of scope” just for the sake of wanting to be conservative. Except TypeScript was made only for the TypeScript team.

The system is changing! If need be that some principles are reviewed as the system changes, to better support and adapt these changes then so be it. Otherwise the system shouldn’t change and fundamental principles be kept.

Moreover, @arendjr’s arguments in summary has not even changed the principles it has only broadened the short-sighted interpretations and undue strictness of these principles.

Oh and I think it is being put to vote already seeing how many reactions are yet in favor of @arendjr’s opinion.

@fabis94
Copy link

fabis94 commented May 25, 2022

I hope the TS team reevaluates their stance, the points made in the OP definitely make a lot of sense

@jakubmazanec
Copy link

jakubmazanec commented May 29, 2022

Technical aside, this issue make the DX worse for everyone. It doesn't make any sense for relative import to refer to a non-existent .js file.

As @s123121 said, the DX currently is horrible. If you're using anything else other than TSC for transpiling *.ts files, node16 is unusable, because other tools (e.g. Parcel, Jest, etc.) don't understand why are you trying to import non-existent file.

@unional
Copy link
Contributor

unional commented Jun 2, 2022

Another argument about file extension:

The purpose of the file extension is to explicitly state which file to resolve to.

In type: module, the CommonJS should have extension .cjs, while ESM have extension .mjs

Which means, if I am writing the code in TypeScript, and have two tsconfig to create those two outputs, then by definition tsc must rewrite the file path during compile time, regardless if I specify the file extension in TypeScript, and if I specify it as .ts, .cts, .mts, .js, .cjs, or .mjs.

Currently, the following is still not possible for TypeScript packages:

"exports": {
  "import": "./esm/index.mjs",
  "require": "./cjs/index.cjs"
}

@weswigham weswigham added the Duplicate label Jun 2, 2022
@weswigham
Copy link
Member

weswigham commented Jun 2, 2022

For anyone who stumbled upon this, if you want .cjs output files, use .cts input files, likewise if you want .mjs output files, use .mts input files. We've landed in a very nice place where there is a 1:1 mapping between input file extension and output file extension (excepting tsx), which is very very helpful for a number of aspects in tooling (like actually being able to find declaration files for arbitrary code without extra configuration).

Which means, if I am writing the code in TypeScript, and have two tsconfig to create those two outputs, then by definition tsc must rewrite the file path during compile time,

Keep your two tsconfigs with two outDirs, use .js extensions everywhere, in addition to a root package.json with type: module, put a sub-package.json in your esm output folder specifying type: module, and one in your cjs output folder specifying type: commonjs. Run your cjs build with module: commonjs, and your esm build with module: nodenext. No extension rewriting required, .js everywhere. The two separate builds are important because they're highly liable to typecheck differently - the esm and cjs modules will import different things (this is a feature in node, not our behavior), and writing a single source codebase that typechecks in both is actually incredibly difficult in practice, so I wouldn't recommend it. It's much easier to write a single source (mostly cjs but using module: nodenext) module and hand-write specific cjs and esm entrypoints as needed (or only write esm).

@unional
Copy link
Contributor

unional commented Jun 2, 2022

Keep your two tsconfigs with two outDirs, use .js extensions everywhere, in addition to a root package.json with type: module, put a sub-package.json in your esm output folder specifying type: module, and one in your cjs output folder specifying type: commonjs

Um... that doesn't work in practice, AFAIK.

I was planning to migrate my packages to type: module.

Originally, I was planning to have a clean cut of migrating from CommonJS to ESM.

i.e., from:

{
  "main": "./lib/index.js"
}

to:

{
  "exports": {
    "import": "./lib/index.js"
  }
}

I completely subscribe to that idea, and understand that is the way to go as:

the esm and cjs modules will import different things

But what I have found is that when I publish the package like that, it breaks everywhere.

Because of the interconnected dependencies and the lovely node module resolution algorithms.

So the migration needs to be done in two phases:

First, migrate to:

{
  "exports": {
    "import": "./esm/index.js",
    "require": "./cjs/index.js"
  },
  "main": "./cjs/index.js"
}

And make sure everything works (I have 100+ open source packages).

Then, they can be migrated to ESM only packages.

Having two package.json doesn't really work, when you are talking about publishing to NPM,
unless you abundant the original package.

i.e. package-a -> package-a-cjs and package-a-esm.

@weswigham
Copy link
Member

weswigham commented Jun 2, 2022

Having two package.json doesn't really work, when you are talking about publishing to NPM,
unless you abundant the original package.

Nested package.json files within a package work fine? The sub-ones doesn't even have to contain anything other than the type field to provide resolution metadata for node itself. They're not for declaring two separate packages - they're for declaring the format for the .js files within those folders. There's still a single top level package.json for the package itself.

@weswigham
Copy link
Member

weswigham commented Jun 2, 2022

And make sure everything works (I have 100+ open source packages).

Then, they can be migrated to ESM only packages.

Dropping cjs support is still definitely going to be a major-version-bump change, even if adding dedicated esm entrypoints can be done in a minor.

@unional
Copy link
Contributor

unional commented Jun 3, 2022

There's still a single top level package.json for the package itself.

That's interesting. Maybe something I need to look into.

Dropping cjs support is still definitely going to be a major-version-bump change, even if adding dedicated esm entrypoints can be done in a minor.

Yes, definitely.

@arendjr
Copy link
Author

arendjr commented Jun 3, 2022

@weswigham Could you please elaborate on the Duplicate label you added? I see no reference to any issue this would be a duplicate of, nor am I aware of any such issue. Of course the generic topic of rewriting import paths has been discussed before, but certainly not in this context.

As an aside, I sorted your issues by number of upvotes and it appears that in three weeks time, this issue has become (with a wide margin) the most upvoted issue of the past year. If this were truly a duplicate, it seems the original was not resolved well.

@weswigham
Copy link
Member

weswigham commented Jun 3, 2022

Of course the generic topic of rewriting import paths has been discussed before, but certainly not in this context.

Duplicate of #16577 - "this context" doesn't really meaningfully change anything - if anything it makes rewriting extensions even less appealing by the logic we've stated before, since now we have multiple input extensions that already mean multiple output extensions.

We're not trying to somehow make node's new behaviors more paletable for people - that's node's fault, not ours. Our philosophy has been, and will be, that we don't semantically change your code during emit, and rewriting imports most certainly is a semantic change (a semantic change that, as I've stated elsewhere, we could never get right 100% of the time thanks to dynamic import operations).

If you really want omitting extensions to be valid node esm, (as it is in cjs) I suggest a complaint about the new node behavior to require extensions over at nodejs/modules#323 or on the node repo proper, rather than here, which is pretty much invisible to the node maintainers - as whatever node supports, we will follow suit.

@arendjr
Copy link
Author

arendjr commented Jun 3, 2022

@weswigham I'm sorry, but it appears you do not entirely understand what we're asking for here.

Our philosophy has been, and will be, that we don't semantically change your code during emit, and rewriting imports most certainly is a semantic change

If that it is truly your philosophy, you've surely been very selective at following it. Rewriting ESM to CommonJS by itself is a semantic change, yet you have no philosophical issues implementing it. The esModuleInterop configuration option semantically alters how the code should be generated, yet you have no qualms with that.

(a semantic change that, as I've stated elsewhere, we could never get right 100% of the time thanks to dynamic import operations)

We're not asking for dynamic imports to be supported here. Raising that as an issue is a strawman argument, because dynamic imports themselves are fundamentally different from static imports, and we can understand those to be out of scope. Hence why that's not what's being asked for here.

If you really want omitting extensions to be valid node esm

Once more, I'm not asking for the omission of extensions, I'm asking for the rewrite of extensions in the exact way that is compatible with how your build process already rewrites extensions. This is not a duplicate of the issue you reference, it's a different request.

@mlandalv
Copy link

mlandalv commented Jun 3, 2022

If you really want omitting extensions to be valid node esm

I'm not sure anyone is asking for this. We're talking typescript and at least I don't consider my TS code to be valid esm/cjs until it's transpiled. Sorry deno land folks (although I would expect .ts to work).

// utils.ts
export const sum = (a: number, b: number) => {
    return a + b;
};

// index.cts
import { sum } from './utils';

console.log(`CJS 1+2=${sum(1, 2)}`);

I'm not considering index.cts to be valid cjs until it's transpiled and obviously it's not since it doesn't use require. But still it works and transpiles properly.

// index.ts
import { sum } from './utils';

console.log(`CJS 1+2=${sum(1, 2)}`);

Just the same, I would expect

tsc --module CommonJS --outDir dist/cjs src/index.ts

to output valid commonjs, and

tsc --module NodeNext --outDir dist/esm src/index.ts

to output valid esm.

And I would gladly add .ts extensions if that made things less ambiguous.

Also it's quite unintuitive, or confusing, that package.json#type overrides/changes the behavior of tsconfig.json#compilerOptions.module. I thought there was a bug in tsc until I realized I had to use type = module to get anything other than commonjs (4.7.x). Byt type = module breaks lots of other stuff in my projects, so not really ready to change that just yet.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jul 11, 2022

I very rarely encounter imports like "foo/bar.ts". So if that case was an error, that would be fine with me.

There are many, many projects that only use paths of this form, e.g. VS Code, or monorepos that permit deep linking.

@whitetrefoil
Copy link

whitetrefoil commented Jul 11, 2022

Ummm, about "./my-file.ts" vs "./my-file.js", I have a maybe a little bit stupid question...
If I have a "./my-file.ts" & a "./my-file.tsx" in the same dir, how to specify which file to import?
I know I can config it in tsconfig.json, but that's project wise...

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jul 11, 2022

If I have a "./my-file.ts" & a "./my-file.tsx" in the same dir, how to specify which file to import?

How do you specify which file to import at runtime?

@bakkot
Copy link
Contributor

bakkot commented Jul 11, 2022

There are many, many projects that only use paths of this form, e.g. VS Code, or monorepos that permit deep linking.

I think such projects are probably a small fraction of all projects?

In any case, if it's possible to make the experience nicer for many common cases, it seems a shame to say that we can't do that because there are other cases where it does not help as much.

@whitetrefoil
Copy link

whitetrefoil commented Jul 11, 2022

If I have a "./my-file.ts" & a "./my-file.tsx" in the same dir, how to specify which file to import?

How do you specify which file to import at runtime?

Umm... I remember ts-node / ts-loader w/o "files" option will resolve files through imports instead of "includes" in tsconfig. So only the specified file will go into runtime (or bundle time actually).

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jul 11, 2022

So only the specified file will go into runtime (or bundle time actually).

Then you're looking for #37582, not this feature

@whitetrefoil
Copy link

whitetrefoil commented Jul 12, 2022

So only the specified file will go into runtime (or bundle time actually).

Then you're looking for #37582, not this feature

Yeah, you're right. I'm tracking both issues and got a little bit confused since they're related.

@bakkot
Copy link
Contributor

bakkot commented Jul 14, 2022

In support of my claim that using the name of a file which doesn't exist is confusing, especially for people who are just trying to get something simple to work and not mess around with massive monorepos or whatever, see e.g. here.

You will not be able to make this not-confusing with documentation or a blog post, because most people are not going to read the documentation or the blog post before encountering the confusing behavior and because even when reading the documentation it reads as a hack rather than as something which is supposed to be the happy path.

I appreciate where you're coming from with the hardline stance against rewriting module specifiers. I can understand the concern when there's complicated paths like import "a/b/c" which some build system is going to need to process with nontrivial resolution logic. But when the module specifier is a relative path to file which is also being compiled by this invocation of tsc, I really do not think the benefits of sticking absolutely to this principle are worth the costs of confusing people who are trying to do something which ought to be straightforward.

I think there's a case to be made for treating module specifiers as being unlike other runtime features, because when TypeScript is acting as a compiler it is in fact doing two things: 1) removing types and 2) outputting a file with a different name. Module specifiers are unique among runtime features in that they actually do interact with one of these things - no runtime features can interact with types and no other runtime feature refers to file names (outside of stuff like import.meta.url which can't be statically rewritten anyway). So I don't think the argument for leaving module specifiers alone is as strong as it is for leaving other runtime features alone.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jul 14, 2022

People regularly understand that when they fs.readFile, they are allowed to only read files that will exist in the output folder of their app, and they don't expect their compiler to rewrite some of the input file paths in that situation, nor should they.

Is this wrong? Should we be detecting calls to fs.readFile where it looks like the input is a relative filename and rewriting that, too? If not, how to explain the difference?

@bakkot
Copy link
Contributor

bakkot commented Jul 14, 2022

If not, how to explain the difference?

One of these things is a syntactic feature, one of them is not.

Also, one of them is being compiled right now (after all, you are importing types with the import, and those types are in the .ts file but not in the .js file), and is the domain of the compiler (because the compiler does module resolution at build time, and not at runtime), and so many people would naturally assume that the compiler is going to rewrite that one.


You're asking this question as if you think it's a difference which is actually going to be surprising to people. And I can see how one could look at this and think these things are similar. But my claim is that in practice, people see import x from './foo.ts' and expect that to be rewritten, and do not similarly see fs.readFile('./build/foo.json') and expect that to be rewritten (at least not nearly as often), whatever the underlying similarity of these two things might be.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jul 14, 2022

Which category does dynamic import clearly fall under?

if (fs.existsSync("./foo.ts")) {
  const p = await import("./foo.ts");
}

@bakkot
Copy link
Contributor

bakkot commented Jul 14, 2022

Can you import types with dynamic import? If so, the compiler's domain, I would think. If not, not.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jul 14, 2022

You're saying that if we added the ability to import types with dynamic import, we would start rewriting paths that we previously didn't?

@bakkot
Copy link
Contributor

bakkot commented Jul 14, 2022

I would be extremely surprised if TypeScript added the ability to import types with dynamic import. What would that even mean?

But yes, I am saying that if I saw type { x } = await import('./foo.ts'), or whatever, then I'd expect that to get rewritten (well, actually, I'd expect it to get eliminated, the same way that import type { x } from './foo.ts' gets eliminated even though that potentially means eliminating side-effects, but assuming there were some syntax which dynamically imported both a type and a non-type).

If you can import types with this syntax, then it's definitionally not just runtime syntax, in the same way that types are not runtime syntax, and hence is something I'd expect the compiler to handle.

("start rewriting paths we previously didn't" sounds dramatic, but a.) this is not actually going to happen, because importing compile-time types with a runtime-specified import is incoherent and b.) even if it did, very few people will have written import('./foo.ts') in a file which is to be compiled by tsc, because ./foo.ts is not going to exist in the output, so very few people would need to deal with such a hypothetical breaking change.)

@arendjr
Copy link
Author

arendjr commented Jul 14, 2022

Dynamic imports should not be rewritten because their resolution happens — by design — at runtime. Static imports were designed to be statically resolved, which many bundlers use to their advantage to eliminate them entirely. Static resolution happens at compile time rather than runtime and should thus resolve to source files.

You're saying that if we added the ability to import types with dynamic import, we would start rewriting paths that we previously didn't?

To put this argument back at you: If tsc one day finally adds the ability to bundle output files for better performance, like other bundlers, will you finally start using source paths because there’s no output to refer to?

@boblauer
Copy link

boblauer commented Jul 14, 2022

It seems fairly obvious that the line in the sand is top-level imports vs everything else. If it's not import x from y.ts, it's everything else, and it should not be rewritten.

Is the file resolved at compile time or run time? If compile time then rewrite the extension, otherwise don't. That's it. You keep bringing up dynamic imports but the people in this thread could not have been more clear that we're not talking about dynamic imports.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jul 14, 2022

I would be extremely surprised if TypeScript added the ability to import types with dynamic import. What would that even mean?

You can already write

type A = typeof import("./a")

To put this argument back at you: If tsc one day finally adds the ability to bundle output files for better performance, like other bundlers, will you finally start using source paths because there’s no output to refer to?

This is exactly what #37582 is about - that we should allow the use of .ts file extensions, which I've repeatedly said we should do. Whether or not a path is "rewritten" in this scenario is completely moot, it's like asking what color zero cows are. There are no source-side paths appearing in the output in this scenario so there is nothing to rewrite.

It seems fairly obvious that the line in the sand is top-level imports vs everything else. If it's not import x from y.ts, it's everything else, and it should not be rewritten.

It's apparently not obvious at all. The distinction @bakkot defined was that top-level import is syntax vs fs.readFile is a function call, but dynamic import is clearly syntax, not a function call. So already there's disagreement here on a fairly straightforward question of what this feature would do.

we're not talking about dynamic imports.

You're not, but that doesn't mean no one else will. It's our job to think about more cases than the ones being brought up because people will expect all follow-on behavior from those initial cases to be consistent and correct. It's very easy to propose something that works well in a narrow set of use cases and completely falls apart into chaos when you go outside that line, but people will disagree on what the boundaries of that path are. People do use dynamic import and they will expect any "rewriting" schema to have reasonable and consistent behavior.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jul 14, 2022

Is the file resolved at compile time or run time? If compile time then rewrite the extension, otherwise don't.

I don't understand this because the answer is almost always "both"? TypeScript will locate the target of a dynamic import for type analysis. The runtime will load it when the expression is evaluated.

@arendjr
Copy link
Author

arendjr commented Jul 14, 2022

Whether or not a path is "rewritten" in this scenario is completely moot, it's like asking what color zero cows are. There are no source-side paths appearing in the output in this scenario so there is nothing to rewrite.

Note how I didn’t ask whether you would start rewriting paths. You’re replying to a point I didn’t make. I asked if you would start using source paths. Because that would be a breaking change for all your users in the same vain as starting to rewrite paths for dynamic imports would be: Everyone would need to update their sources before they can opt-in. And that’s why it’s so unfortunate that TypeScript seems married to the idea of using output paths… the implementation of the bundler/compiler defines what kind of paths users have to use, forcing them to understand those compiler/bundler implementation details as well as tying their code to that implementation rather than making it portable where it could be.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jul 14, 2022

If we allowed you to write .ts file extensions, then by logical extension we would be letting you write source paths whether or not bundling exists.

@bakkot
Copy link
Contributor

bakkot commented Jul 14, 2022

You can already write

type A = typeof import("./a")

That's not a dynamic import. That's purely type-level syntax which happens to look like a dynamic import.

If you want to say that's an actual dynamic import, then the fact that it gets eliminated entirely is an example where TypeScript is already changing the behavior of non-type-level code.

The distinction @bakkot defined was that top-level import is syntax vs fs.readFile is a function call, but dynamic import is clearly syntax, not a function call

Sorry, "syntactic" was speaking loosely. Function calls are syntax just as much as dynamic imports are. "Resolved at runtime" vs "resolved at compile time" is the relevant distinction.

Also, it doesn't actually matter what mental models people have as long as they give the same answers, and both of us agree that import './x.ts' would be rewritten while readFile('./x.ts') would not, so there's not an actually relevant disagreement between us.

And yes, module specifiers are resolved both at compile time and at runtime. The point of this thread, though, is that different files exist in each phase - that is, after all, what using tsc as a compiler does. So the ask is to write module specifiers as they exist at compile time in the files which exist at compile time, and have the compiler translate those specifiers to the files which exist at runtime in the files it is outputting for consumption at runtime. At least in simple, common cases.

people will disagree on what the boundaries of that path are

I agree there's going to be edge cases. But it seems like almost every expects top-level imports of relative paths to get rewritten. It seems silly to say that the fact that there will always be some cases where not everyone will agree with whatever choice is made means we must also preserve the cases where almost everyone will disagree with the choice being made. (Should voting age be 16, 18, or 21? Reasonable people can disagree, but that doesn't mean it should be 0.)

Yes, "never rewrite any specifiers" is a consistent rule for when to rewrite module specifiers, from one point of view. But it's not consistent in a way that is unsurprising. It's surprising to everyone, and it's surprising in literally the most common case, not just in edge cases. So what good is this consistency doing?

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jul 14, 2022

It's surprising to everyone, and it's surprising in literally the most common case, not just in edge cases. So what good is this consistency doing?

Once you violate the consistency, you open up a new world of expectation-vs-reality breaks where people have to understand exactly how and where the consistency is violated. This doesn't eliminate the problem, it just spreads it around -- dynamic imports, nonrelative imports, the exact rules for how the extension gets changed (e.g. tsx becomes jsx or js depending on what?), and so on.

I will never claim that it is not a trade-off -- consistency is often a false proxy for some other more important measure. But I think taking a principle that currently has 100% consistency and breaking that would be a huge step backwards. Today we have a mental model we can give to people about how their JS is created from their TS. If we start rewriting some paths but not others, the questions of which and how become enormous tarpits of complexity that people will be forced to understand in order to write programs that simply run at all.

The status quo is subject to a single initial surprise - this path got left as-is - but in return, there are exactly zero more surprises in that area for the rest of your life. If some paths get rewritten, there is now an unbounded number of future surprises in terms of incorrect guesses about whether and how any given path in your program gets rewritten.

@bakkot
Copy link
Contributor

bakkot commented Jul 14, 2022

you open up a new world of expectation-vs-reality breaks where people have to understand exactly how and where the consistency is violated. This doesn't eliminate the problem, it just spreads it around -- dynamic imports, nonrelative imports, the exact rules for how the extension gets changed

Have you actually checked to see whether these questions are ones on which people have strongly held, divergent intuitions? It might be the case, and that would be unfortunate although not fatal, but I don't think even this has been firmly established. (If people don't have strongly held intuitions - I certainly don't have a strongly held intuition about how nonrelative paths work at all, much less the rules for rewriting them - then there's not much in the way of expectations to be violated in the first place.)

This is a question you can investigate before giving up! Not in this thread, which will get a biased sample, but you can post a poll somewhere along the lines of "Assuming TS decides that import './foo.ts' should be rewritten to import './foo.js', what do you think let x = await import('./foo.ts') should get rewritten to? A) left alone as './foo.ts', B) rewritten to './foo.js', c) I don't have a strongly held intuition."

(Edit to add: also, have you spent any time trying to come up a consistent if possibly slightly longer rule which would cover the simple cases? For example, "only static imports of relative paths are rewritten" really isn't very much harder to learn than "no paths are rewritten". The second thing is marginally shorter but, I claim, more surprising; shortness of the rule is only a proxy for surprisingness, which is the important quality.)

Today we have a mental model we can give to people about how their JS is created from their TS. If we start rewriting some paths but not others, the questions of which and how become enormous tarpits of complexity that people will be forced to understand in order to write programs that simply run at all.

I think you are substantially overestimating the complexity here. There's not that many questions to be answered, and the more obscure questions a.) aren't relevant to most people and b.) are precisely the ones on which people are least likely to have a pre-existing intuition.

And I think you are also weighting these incorrectly. I think the additional complexity encountered just by people trying to read or write TypeScript for the first time, in the status quo, is going to substantially outweigh the additional complexity encountered by people running into any of these edge cases which break with their expectations.

The status quo is subject to a single initial surprise - this path got left as-is - but in return, there are exactly zero more surprises in that area for the rest of your life.

Only once people have actually internalized this rule. The problem is that the rule is so surprising that, in fact, it is going to trip people up repeatedly. I know I've already gotten burned more than once.

Yes, once you've been burned half a dozen times you might remember the next time, but I don't think this is a good learning experience. And this is a cost paid by beginners, rather than a cost paid by people who have spent enough time doing weird stuff to be running into edge cases. Most beginners are not contributing to VSCode, so the fact that VSCode does some weird nonrelative import shenanigans where the rule might not be obvious simply is not relevant to them.

@arendjr
Copy link
Author

arendjr commented Jul 14, 2022

If we allowed you to write .ts file extensions, then by logical extension we would be letting you write source paths whether or not bundling exists.

That’s true. Unfortunately though, tsc is still the reference implementation and it does not allow it. Which is why we end up with such unfortunate situations where bundlers such as Esbuild and Vite bend over backwards to imitate your implementation, despite it not making sense for them other than to accommodate for both users and library developers that don’t really have a grasp anymore of how things are expected to work.

I opened this issue before ”node16” was stabilized in hopes to avoid this mess the ecosystem is becoming now, but alas…

@arendjr
Copy link
Author

arendjr commented Jul 14, 2022

The status quo is subject to a single initial surprise - this path got left as-is - but in return, there are exactly zero more surprises in that area for the rest of your life. If some paths get rewritten, there is now an unbounded number of future surprises in terms of incorrect guesses about whether and how any given path in your program gets rewritten.

Funny how you get this exactly backwards, btw. Today, it is the user’s problem to know how and whether paths get rewritten. They have to know the output extension, and based on which config setting it gets chosen, because you make them type it out. If you would rewrite the paths for them, it’s no longer their concern and they don’t have to care how paths get rewritten. You’d be taking away surprises, as the many people voting for this issue can attest.

Unfortunately you seem so stuck on focusing on this one “consistency” issue that it seems you will reason backwards to get at your own conclusion no matter what. All the other inconsistencies I’ve pointed out in earlier comments get swept under the rug. Because those are not the inconsistencies you care about. I think you said it right when you said “consistency is often a false proxy for some other more important measure”.

@bakkot
Copy link
Contributor

bakkot commented Jul 14, 2022

@arendjr

Unfortunately you seem so stuck on focusing on this one “consistency” issue that it seems you will reason backwards to get at your own conclusion no matter what. All the other inconsistencies I’ve pointed out in earlier comments get swept under the rug. Because those are not the inconsistencies you care about. I think you said it right when you said “consistency is often a false proxy for some other more important measure”.

This is a bit off-topic, but, speaking as someone who agrees with you about the general direction I would like TS to take on this topic, this sort of rhetoric really is counterproductive (as well as being impolite). I'm confident that everyone here is arguing in good faith for positions they sincerely support on grounds which feel justified, and are neither deliberately ignoring anything nor reasoning backwards from conclusions they are committed to in face of all reason. That @RyanCavanaugh is discussing this at all is evidence enough of that; maintainers are under no obligation to hear us out or to try to explain where they're coming from.

And speaking as someone who's been on the receiving end of this sort of comment often enough, as a maintainer reading this sort of thing would very much make me want to disengage from this conversation.

@arendjr
Copy link
Author

arendjr commented Jul 14, 2022

I apologize for the tone in my earlier comment. I admit I’ve been soured with how earlier arguments have been ignored in this thread. I personally do find the representation of rewriting paths as “ enormous tarpits of complexity” to be disingenuous, but it was not my intent to discourage anyone arguing in good faith.

@demurgos
Copy link

demurgos commented Aug 1, 2022

Webpack 5.74 added native support for extension aliases. It means that you can use .js and it will resolve the .ts file (same behavior as tsc). You no longer need a plugin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate Out of Scope Suggestion
Projects
None yet
Development

No branches or pull requests