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

Design Meeting Notes, 11/22/2019 #35589

Closed
DanielRosenwasser opened this issue Dec 9, 2019 · 60 comments
Closed

Design Meeting Notes, 11/22/2019 #35589

DanielRosenwasser opened this issue Dec 9, 2019 · 60 comments
Labels
Design Notes Notes from our design meetings

Comments

@DanielRosenwasser
Copy link
Member

.mjs Input Files

#27957

  • Node.js shipped module support recently

  • How's this work?

    • In the Node runtime, as a CommonJS consumer, you can't interact with ESM at all.
    • require always does a CJS resolution
    • In a module, you can only import other modules, and they need to have extensions.
  • Idea: if what Node has today is the best that it will ever deliver...

    • We'd have to redo basically everything in the CJS resolution scheme we do.
    • We'd need to add .mjs resolution, but error on that.
    • There will need to be a module format output.
    • Also, import(...) would...need to not be imported.
  • Wait, why would you want any of this?

    • You want to migrate consuming code (?)
  • Should we emit .mjs files?

    • Shouldn't emit .ts files to .mjs files because that'd be a massive breaking change.
    • Need a new module field?
    • Need .mts and .cts?
      • .mtsx and .ctsx?
      • .d.mts and .d.cts?
    • This is absolutely unreasonable.
      • Need to be able to disambiguate format of the original inputs.
      • Could it be in-band?
        • File extensions mean you don't need to read the file contents.
  • As long as we don't rewrite imports (and we won't because it's error prone and requires whole-program knowledge), then we need the disambiguators.

  • Take the following example

    foo/package.json
        foo/index.js   (this is ESM)
        foo/index.cjs  (this is CJS)
    
    • require("foo") breaks because it'll resolve to foo/index.js
    • import "foo" doesn't work because it won't resolve to /index.js because Node thinks it's too magical for ESM.
  • This logic is extremely complex.

    • "The combinatorics of this are extremely absurd."
    • Can we come up with a smaller set of what is supported?
      • By doing that, it provides a prescriptive direction for users who ask "how do I write ES modules in Node and TypeScript?"
    • But have to be smart about what we "leave on the table" and what that breaks.
      • Also want to leave things open for later additions.
  • Conclusion:

    • .mjs in an import path is doable long-term, but not
    • Node ESM support as a whole will need to be scoped out.
      • .mjs output is troublesome.

--emitExtensions and Importing .ts Files

#35148

  • We're not rewriting paths.
    • Not now, not tomorrow, not ever.
  • So is fixing this just a matter of relaxing the check?
    • Sounds like it.
    • But the check stops people from shooting themselves in their feet, so don't want to just confuse people more.
  • Unclear what the original use-case really is. Linked issues have very different goals/use-cases.
  • It sounds like the only use-case is deno
    • So "moduleResolution": "deno"?
    • Uh, but what's your emit??
    • How do you deal with https:// paths?
      • Maybe don't.
      • Can simulate this with path mapping/import maps.

PnP Resolution

#35206

  • What are the problems with pnp?
    • Executes arbitrary .js file.
    • The defaults make little sense in the editor - means that users need to configure their editors to load tsserver with yarn. Very opt-in.
    • Also, while guidance is to stick to one version of package manager, users sometimes mix/match package managers and different versions.
    • Kind of hard to be sure what doing the right thing means here that doesn't require the same level of configuration as an LS plugin that overrides resolution

Reorder Extension Priorities

#34713

  • Out of time.
@DanielRosenwasser DanielRosenwasser added the Design Notes Notes from our design meetings label Dec 9, 2019
@MicahZoltu
Copy link
Contributor

We're not rewriting paths.
Not now, not tomorrow, not ever.

I have seen this several times but haven't seen details about it. Can someone help explain it for us plebs why this is "100% off the table, never to be considered, never to be discussed, never to be brought up"?

I suspect there is a good reason for this, but it would really help me accept it if I could read the discussion that lead to such a hardline stance. I understand it is hard, but given the other options it feels like we really should be exploring all possibilities fully, including hard ones.

@Jack-Works
Copy link
Contributor

We're not rewriting paths.
Not now, not tomorrow, not ever.

I have seen this several times but haven't seen details about it.

Agree, if not rewriting the extension name, then how to emit valid code for Node module resolution and deno?

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Dec 11, 2019

I have seen this several times but haven't seen details about it. Can someone help explain it for us plebs why this is "100% off the table, never to be considered, never to be discussed, never to be brought up"?

Let's say you write some code in a JavaScript file

var x = y + z;

If you paste that code into a TypeScript file, it means

var x = y + z;

This the fundamental promise of TypeScript: You can take some JavaScript code, put it in a TypeScript file, and it means the same thing meant before. I would argue it is the most important design principle we have, because it's the only reason JS -> TS migration is possible and the only reason JS <-> TS interop is sane.

This is why we don't have extension methods, even though it would be nice.
This is why we don't have operator overloading, even though it would be nice.
This is why we don't have exceptions on missing property access, even though it would be nice.
This is why we don't have automatic binding of this on class methods, even though it would be nice.
This is why we don't have native integers, even though it would be nice.
This is why we don't have exceptions on divide-by-zero, even though it would be nice.
This is why we don't have automatic prefixing of this. on class members, even though it would be very nice.

We've kept that promise on literally any random JS snippet you can think of, and while some JS code may have type errors, it does the same thing at runtime it does before. Changing your file extension from .js to .ts will not break your program.

Now you can say "Well that promise sucks, you should break it and just rewrite paths because rewriting paths is the best possible thing a programming language can provide". OK, fine.

Our stance has always been that you should write the import path you want to appear in the emitted code, and add path mapping or other configuration to make TypeScript understand what that path should resolve to. This is 100.0% consistent with the idea that you should write the JavaScript you want to appear in the emitted code and add type annotations or assertions to make TypeScript understand what the type intent of the code is. In fact, it's not even a different principle at all, because an import statement is JavaScript and we have already established that you should write the JavaScript you want to be emitted.

The thing I see every time is that people go through this cycle:

  1. Start with working import paths
  2. Add path mapping to their tsconfig to create aliases
  3. Change their import paths to use those aliases
  4. The program stops working because the import paths are now wrong

The right fix is to either:

  • Not do step 2 in the first place
  • Tell your loader about the aliases you invented in step 2 so it can handle them

TypeScript isn't here to provide a module aliasing system. If your loader supports such a system, great, use path mapping to describe it. TypeScript isn't obligated to invent new ways of adding epicycles to module resolution, and doing so would violate one of our core principles.

But again, let's say you think that rule is bad. What happens if we start rewriting import paths tomorrow?

Well, today, you write the import path that you want to appear in the emitted JavaScript file. You have two paths to consider here:

  • The import path you wrote, which you can trivially understand will appear in the output
  • The file that TypeScript resolved this to

This is already insanely complicated. paths, baseUrl, baseUrls, the module resolution setting, the location of the originating file, the effect of symlinks, file extension priorities, the fact that there are really three different kinds of paths each with their own independent resolution strategies, etc. etc..

Tomorrow, if you write an import path, you now have three things under consideration:

  • The import path you wrote
  • The file TypeScript resolved that to
  • What the path that should appear in the output is

This is taking a complicated 2-dimensional math problem and moving it to 3 dimensions. People can already barely understand the interaction of include, exclude, and module imports - do we really want to make this the most complicated configuration system ever imagined? The user confusion will be unending.

That said, this is 0% about the difficulty of such a system. The existing resolution system is already difficult; we are not afraid of doing difficult things. We are opposed to doing things that violate our fundamental promise that the JS code you wrote is the JS code you get.

If you find yourself trying to write a valid runtime path, and can't get TypeScript to understand how to resolve that path to the target module, tell us about it. We have half a dozen module resolution flags and will keep adding more until any valid runtime path you write can be reasonably resolved to a corresponding file or declaration.

Conversely, don't tell us that you wrote an invalid runtime path and want us to fix it for you! This is the same as saying that you wrote var x = y + z; and want TypeScript to emit something different because you really wanted some other JavaScript - that's not what we do, and not what we've ever done.

@Jack-Works
Copy link
Contributor

I got the principal now, and I propose a change in my --emitExtension PR at #35148 (comment)
Does it seems okay now?

@Jamesernator
Copy link

My summary on Node ES Modules and TypeScript incompatibilities

Dynamic modules / Named imports from CommonJS

Currently import * as React from 'react'; and import { PureComponent } from 'react'; as handled by default by TypeScript is not compatible.

Only import React from 'react'; is supported (the whole CJS module as default export).

Why is this the case?

Dynamic modules has not gained consensus within the working group.

Alternative solutions have not been accepted either.

TS Author compromise

esModuleInterop can be enabled for users wishing to output ESM.

Problems with compromise

import * as React from 'react'/import { PureComponent } from 'react'; is not treated as an error but will error in Node's implementation when targeting "module": "esnext".

Required extensions

Extensions are required, current auto-suggestions do not add extensions automatically and still report the code as OK which will definitely cause confusion.

Why is this the case?

Extensionless imports failed to gain consensus although discussion is still ongoing.

TS Author compromise

Authors can add extensions themselves for ES output.

Problems with compromise

Authors must take care to add another package.json if they have multiple targets as .js cannot be used for both targets.

package.json exports

New feature typescript doesn't understand. Basically a package can be published with a package.json like:

{
  "name": "my-package",
  "exports": {
    "/foo": "./dist/foo.js"
  }
}

And consumers can then do:

import foo from "my-package/foo";

Why is this the case?

New feature to support authors of packages to freely change their package structure without affecting consumers.

TS Author Compromise

Only import foo from './node_modules/my-package/dist/foo.js' works for now.

Problems with compromise

This is different to actual supported "my-package/foo.js", feature simply can't be used without TS support.

.mjs/.cjs/"type": "module"

Currently TypeScript uses existence of import/export to distinguish modules and comnmonjs.

Node however uses package.json "type": "module" | "commonjs" to determine whether or not .js is CJS or ESM. .cjs and .mjs are always treated as CJS and ESM respectively regardless of "type".

Some already existing modules cannot be used in TypeScript because of this (e.g. idlize).

Why is this the case?

Automatic detection was rejected due to various hazards. I can't find the actual reference for when this decision was finalized.

Instead dual packages will be replaced with (the currently experimental) conditional exports if no require(esm) or other such solution is resolved by January 2020.

TS Author Compromise

There's no compromise if author's want to use .mjs/.cjs authored packages. Currently import IdleValue from 'idlize/IdleValue.mjs' is simply not supported.

@Jamesernator
Copy link

Jamesernator commented Dec 12, 2019

This is already insanely complicated. paths, baseUrl, baseUrls, the module resolution setting, the location of the originating file, the effect of symlinks, file extension priorities, the fact that there are really three different kinds of paths each with their own independent resolution strategies, etc. etc..

I have never even considered using this features for this reason. However existing complexity doesn't mean the alternative system increases complexity. It may be the case that within roots that have a tsconfig.json with such a feature they cannot use baseUrl/baseUrls/paths/etc.

Speaking for myself, all I really want a is a tool that turns a graph like this:

module-graph

Into one where the .ts files are re-mapped to whatever format I want.

Maybe I want modules:

module-graph

Any maybe I want CommonJS with .cjs to sit alongside side the other modul graph:

module-graph


In these graphs the specifier always points to the type of resource I want, but what I want .ts to be converted to might vary depending on target. Relying on ./noextension doesn't even work if those output graphs go into the same directory like so:

module-graph

In such a graph ./a resolves to multiple but given a resolution order it'll always resolve to one of them. So simply outputting the specifier with no transform doesn't work.

@weswigham
Copy link
Member

I can't find the actual reference for when this decision was finalized.

Because it hasn't been. There are just individuals with opinions - I'm one of them.

@Jamesernator
Copy link

Because it hasn't been. There are just individuals with opinions - I'm one of them.

Well some agreement must've been achieved to unflag the current implementation (even if it is still experimental), this seems like a pretty strong signal that the core implementation of modules is nearing readiness even if there's still rough edges.

Also the problems will still be present even with auto detection, as TypeScript will still need to learn to understand the extensions if it wants to be able to support importing packages that use .mjs.

Although I do think this is one of the lesser issues compared to the others I mentioned as TypeScript authors can always decide just to publish non-mixed types for the time being.

@weswigham
Copy link
Member

It is how it is because it's easier to add extension resolution back in than to remove it. That's what we could agree on.

@jkrems
Copy link

jkrems commented Dec 12, 2019

In the Node runtime, as a CommonJS consumer, you can't interact with ESM at all.

Just to clarify: You can't require ESM but you can import() ESM using dynamic import.

@Jack-Works
Copy link
Contributor

I got the principal now, and I propose a change in my --emitExtension PR at #35148 (comment)
Does it seems okay now?

I have fully rewritten the pr, for any one interested, see #35148

@arcanis
Copy link

arcanis commented Jan 6, 2020

Just saw that PnP had been discussed:

  • What are the problems with pnp?

    • Executes arbitrary .js file.
    • The defaults make little sense in the editor - means that users need to configure their editors to load tsserver with yarn. Very opt-in.
    • Also, while guidance is to stick to one version of package manager, users sometimes mix/match package managers and different versions.
    • Kind of hard to be sure what doing the right thing means here that doesn't require the same level of configuration as an LS plugin that overrides resolution

I'm curious if there are suggestions you have regarding our design that would make it easier to reach a solution (outside of plugins - I mean default support)?

@GeoffreyBooth
Copy link

Apologies for discovering this issue so late, and for my limited understanding of TypeScript’s constraints regarding this issue. Back in 2016 I added support for import and export to CoffeeScript, and we took essentially a “passthrough” approach: CoffeeScript code of import 'foo' was output as JavaScript code of import 'foo', and it was the responsibility of some other tool in the build chain to convert that import 'foo' into require statements or whatever else the user wanted.

For an intended runtime where ES modules are supported, like modern browsers, this works great; the author just needs to write browser-compatible import statements using URLs. For Node 13+ with native ESM, the author needs to write the URLs that Node would run, e.g. import './file.js', and set package.json "type": "module" or set up another build step to rename .js files to .mjs.

I gather from the discussion here that lots of people prefer to have TypeScript be their only build step, so a secondary step for renaming files is undesirable. Has it been considered that the TypeScript compiler have an option to just output import and export statements more or less as written? And then users can follow the pattern I just described, where "type": "module" is set and the original TypeScript contains Node-runnable statements like import './file.js'. Is the issue that TypeScript itself needs to follow the URL in order to load the imported file and get its types, and therefore it needs to file.ts instead of file.js?

@MicahZoltu
Copy link
Contributor

The conflict stems from TypeScript assuming that when you write TS code, the code will target a single runtime environment and you will know the file names/paths at dev time. Many people want a workflow that doesn't know file names/paths until compile time, and the introduction of NodeJS's .mjs filename requirement together with browser's need for explicit full file paths makes it so now users have to choose at dev time what runtime they want to target, rather than waiting until compile time to make the decision (at which point you can do things like multiple builds).

@GeoffreyBooth
Copy link

the introduction of NodeJS's .mjs filename requirement

In the final implementation, Node supports ES modules that use .js. Does that not help this issue?

@MicahZoltu
Copy link
Contributor

In the final implementation, Node supports ES modules that use .js.

I haven't been following this aspect of NodeJS closely, but that statement doesn't align with the comments at the top of this issue (that mjs extension is required).

If NodJS does go with .js extensions for all files (no .mjs) then that mitigates the immediate problem, but still leaves the more general problem that it requires dev-time assumptions about the final output format. For example, imagine a runtime that executes TS natively, or imagine TS compiled down to CLR or JVM bytecode instead of JavaScript. In any of these cases, it would be incorrect to put a .js extension on the module import line because the extension isn't known until compiletime. These tools could all probably be hacked to do compiletime or runtime translation of the .js extension to something else, but I'm not a fan of forcing tooling authors to put hacks in.

@Jack-Works
Copy link
Contributor

Jack-Works commented Jan 17, 2020

I have tried to rewrite extension at compile time in my pr mentioned above (--emitExtension) but typescript team does not accept it.

@weswigham
Copy link
Member

I haven't been following this aspect of NodeJS closely, but that statement doesn't align with the comments at the top of this issue (that mjs extension is required).

Use the "type": "module" field in your package.json.

@GeoffreyBooth
Copy link

I haven't been following this aspect of NodeJS closely, but that statement doesn't align with the comments at the top of this issue (that mjs extension is required).

I’m on the modules team for Node.js. There was an earlier experimental ES modules implementation in Node 7 through 11 that required .mjs, but since 12.0.0 the current implementation lets users use either .mjs or .js. To use .js, users need to add "type": "module" to their package.json: https://nodejs.org/api/esm.html#esm_enabling.

This was added specifically to support file types such as TypeScript and CoffeeScript and JSX that have their own extensions (.ts., .coffee, .jsx, etc.) and compile to JavaScript. Node considers both methods (.mjs or "type") to be equally first-class and fully supported, so you shouldn’t feel like you have to support .mjs if doing so is a burden. Obviously ES modules in .js will require a package.json for the project, so some users might want to be able to output as .mjs in order to save shell script-like files written in TypeScript; but that seems like a minority use case for TypeScript users. There will probably be other build tools that prefer or require .mjs eventually, but you can tackle those cases when they arise.

What issues, if any, are there with TypeScript outputting ES module .js files that Node can run?

@MicahZoltu
Copy link
Contributor

What issues, if any, are there with TypeScript outputting ES module .js files that Node can run?

@GeoffreyBooth Given what you said above about latest NodeJS, it sounds like "type": "module" is the solution for writing a library that works for both browser and NodeJS, though you'll still either need to write in the extension at dev-time (not a problem if browser and node are your only runtimes) or use a TSC transformer, or a post-build transformer, or use a runtime that can do runtime path transformations.

An example of where this is problematic is ts-node, which runs on NodeJS and hooks into the NodeJS module loader. Unfortunately, the NodeJS module loader doesn't callback to external module loaders if the path has a .js extension, so if you write import from './foo.js' ts-node never gets the opportunity to load ./foo.ts so the program will not run. ts-node can work around this by doing path transformations at compilation (which is Just In Time) but that adds a lot of complexity to what is currently just a NodeJS module loader (now it needs to hook in to the TypeScript emit process to change what is emitted).

NodeJS could give external module loaders the ability to hook into JS file module loading (an opportunity to do path transformations) which would solve the immediate problem for ts-node (this may be something you care about) but it doesn't solve the broader issue of making TypeScript (the language) agnostic to runtime environment.

@GeoffreyBooth
Copy link

So ts-node is a whole separate issue, in that it’s currently hooking into Node’s CommonJS loader but now there’s a separate ES module loader; if it wants to support both CommonJS and ES module files then ts-node will need to provide two loaders. We’re still designing an API for hooks for Node’s ES module loader. Transpilation is one of the core use cases, and is already possible via the hooks just merged onto master: https://github.com/nodejs/node/blob/master/doc/api/esm.md#transpiler-loader. Keep in mind that this is very experimental at the moment. If I were ts-node I’d give that work more time to bake before I’d add support for ES modules.

it doesn't solve the broader issue of making TypeScript (the language) agnostic to runtime environment.

I don’t follow. If the user can write import './file.js' in their original TypeScript, and TypeScript will output that unmodified, and Node can run it under "type": "module" (as could any other ESM-supporting environment, from browsers to Deno); then how is this not agnostic to runtime environment?

This gets back to my earlier question. Can a user just write import './file.js' in their original TypeScript, when the only file on disk is file.ts? Will that screw things up in TypeScript’s compilation and type checking, where it tries to find a file.js and can’t?

@MicahZoltu
Copy link
Contributor

Can a user just write import './file.js' in their original TypeScript, when the only file on disk is file.ts? Will that screw things up in TypeScript’s compilation and type checking, where it tries to find a file.js and can’t?

Yes. If you are targeting only browser and NodeJS this is the commonly recommended strategy. If you are targeting things besides those two (like ts-node or some hypothetical future non-JS runtime) then this strategy doesn't work (without hacks) because your emitted files may not have a .js extension. The TypeScript compiler will guess that when you say import from './foo.js' that you probably mean import './foo.ts' for the sake of type definition lookup. This is problematic if you have a foo.js/foo.d.ts and a different foo.ts on disk that doesn't match, but I'll acknowledge that you probably deserve pain and suffering if you do that. 😉 (joking aside, it is common to have index.html, index.css, index.js, so it isn't totally crazy to imagine extension being the only differentiator between files).

@MicahZoltu
Copy link
Contributor

For an example, one can imagine a runtime that runs TS natively (no transpilation to JS). If you were to do import from './foo.js' that wouldn't make sense because there is never a JS file, neither on disk or in memory. The runtime could have hacks that say "if user imports a JS file, ignore what they said and look for a TS file instead", but then what happens when your TS native runtime wants to support loading actual JS files (e.g., for backward compatibility support)? You can introduce more hacks like looking for the file on disk to see which exists... but that doesn't work in environments like browser where checking for existence is expensive.

Currently, TS is agnostic to the target language/file types. At dev time you don't have to make a decision on target runtime, you can put that decision off until compile time or runtime. Requiring that users explicitly provide an extension at dev time breaks that agnosticism and pushes TS toward gnostic language (coupled with the runtime target). This breakage of agnosticism is what I dislike about the current proposed path forward on this issue.

FWIW, I do appreciate that MS has made their stance on this clear, and I respect that they disagree as to the value of runtime agnosticism.

@GeoffreyBooth
Copy link

So I don’t consider ts-node a “runtime,” in the same way that CoffeeScript’s coffee command or Babel’s babel aren’t runtimes. They all just transpile files on the fly before passing them to Node for execution. Node is the runtime, in my opinion. I know it gets a little blurry since Node itself wraps V8, but I don’t commonly see ts-node and coffee etc. grouped along with “true” runtimes like Node and Deno and browsers.

It seems to me that if the TypeScript compiler itself can see file.js and know to look for file.ts instead, or as a fallback if file.js doesn’t exist, then ts-node could do the same. That would seem to me the simplest solution, and the most “correct.” Allowing the user to type import './file' or import './file.ts' is a bit magical, as “real” runtimes like Node or browsers won’t support such specifiers (at least without special configuration like a loader for Node or an unusually configured server for browsers). I would expect that TypeScript authors understand that their code isn’t actually running as TypeScript, but is becoming JavaScript first and being evaluated as that.

@MicahZoltu
Copy link
Contributor

code isn’t actually running as TypeScript, but is becoming JavaScript first and being evaluated as that

If you believe TypeScript should only ever compile to JavaScript and will never be executed natively (without transpilation) and will never compile to anything other than JS (e.g., WASM, native, etc.) then specifying .js extensions in your import statements at dev time is reasonable. If you think that at some point in the future TS will target runtimes other than JS, then specifying the .js extension at dev time is not as reasonable of an option because you may not know the runtime target until compile time.

As an example, Kotlin can compile to JVM bytecode or JavaScript. It is reasonable to write a Kotlin library that isn't runtime specific. If Kotlin required you to specify the file extension of relative imports, you would not be able to author code that worked with either runtime... it would change what is now a compile time decision (runtime target) to a dev time decision.

@ljharb
Copy link
Contributor

ljharb commented May 11, 2020

@weswigham you could wrap all dynamic import paths in a helper that does the rewriting at runtime?

@weswigham
Copy link
Member

you could wrap all dynamic import paths in a helper that does the rewriting at runtime?

Such a large generated runtime component is something we're not interested in. We've tried very hard to keep our helpers minimal we do not provide a runtime, per sey, and don't provide full polyfills ourselves. if you wanted that runtime behavior, you'd need to provide it yourself. Plus, such a helper is inherently falliable in the presence of edge case files with strange chains of extensions like "file.js.ts", when used with cjs style resolution. :(

@Jack-Works
Copy link
Contributor

@weswigham you could wrap all dynamic import paths in a helper that does the rewriting at runtime?

(My custom transformer did that)

@GeoffreyBooth
Copy link

I think for dynamic import() specifiers, the author has opted into needing to know what the hell they're doing, and will need to have figured out what the specifiers will be at runtime and will generate the specifier accordingly. This is also an edge case. I think TypeScript can certainly say that they only rewrite extensions and only for static specifiers. @weswigham?

The way to be sure that there isn't some CommonJS trickery going on like file.js.ts is to check the disk when rewriting: if './file.ts' exists, and the configuration says that file.ts will be output as file.js, then rewrite the './file.ts' specifier to './file.js'. Don't rewrite any extensions otherwise.

@weswigham
Copy link
Member

This is also an edge case

The edges of a system are what define its limits.

You do realize that, under a scheme like that, resolving that specifier went from "it does what it does based on cjs" to "that, except for these cases, except in these scenarios" right? It's a minefield. Especially when you consider cross-project references, and references to (declaration) files that are just straight up are not in the current build. Hell, the declaration file for a file doesn't even encode the original source file's extension in any way (it could be tsx (and therefore jsx) or TS or js or jsx)- it's just associated by location.

This is a minefield we will not enter.

@GeoffreyBooth
Copy link

"it does what it does based on cjs"

But this is the problem. CommonJS-style resolution is a Node-specific thing. TypeScript isn't and shouldn't be defined by Node.

If it were me, I would create a new configuration option called “rewrite extensions of static imports of transpiled files” that does just that: when the transpiler encounters a static specifier of a file that the transpiler itself will be transpiling later (or has already transpiled), that specifier is rewritten. It's that straightforward, and it's not a minefield. Make it an experimental option if you want, and see what edge cases it can't cover. If too many people find too many cases where it doesn't rewrite as they'd expect, drop it. But I'm not persuaded that it's unachievable, or that the definition of when something will be rewritten is so difficult to fathom that users can't grasp it.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented May 11, 2020

I'm not persuaded that it's unachievable, or that the definition of when something will be rewritten is so difficult to fathom that users can't grasp it.

We added an option called exclude that filters what include picks up; with regularity someone shows up and literally insults our intelligence for making this setting not affect the totally unrelated process of module resolution.

A lot of our emit happens through ts.transpileModule which works on one file at a time; all of our regrets are about how this function can't handle all TS programs.

What is a file extension, anyway? Is it unfathomable to see a repo with filenames parser.ts, parser.json.ts, and parser.js.ts ? What does an import of parser.js or parser.json mean in that setting? Is the answer so obvious that no one could ever be surprised? Can you design this in a way that adding a new file to disk never changes the output of "unrelated" files? How does this work in hosted scenarios where TS can't see what's on disk?

The current rule could not possibly be simpler, and cannot possibly break a working program. You'd have to convince us that there's a huge pot of gold on the other side of the rainbow to give that up.

@ljharb
Copy link
Contributor

ljharb commented May 12, 2020

What is a file extension, anyway?

the extension is always and only the final piece (x.slice(x.lastIndexOf('.')), ignoring extensionless files for this pseudocode); that's how node operates. afaik only rails via sprockets has the convention that in .a.b, both the a and b matter

@RyanCavanaugh
Copy link
Member

.d.ts

(┛ಠ_ಠ)┛彡┻━┻

@DanielRosenwasser
Copy link
Member Author

I want to repeat this because it is such a strong point:

A lot of our emit happens through ts.transpileModule which works on one file at a time; all of our regrets are about how this function can't handle all TS programs.

This is the thing - a mode where you always apply a file extension change is wrong because it can't resolve. A program that resolves to determine emit on extensions is fundamentally incompatible with transpileModule and all the single-file-at-a-time JS compilers out there like Babel.

And in either of these modes, "what if the .d.ts files on disk lie?" is a question that people keep hand-waving away. If an import of ./foo resolves to ./foo.d.ts, but is actually ./foo/index.js at runtime, you're hosed either way. You can say "well that .d.ts should reflect the real state of the world, but that's exactly how .d.ts files are often written.

@GeoffreyBooth
Copy link

A lot of our emit happens through ts.transpileModule which works on one file at a time

Forgive my ignorance, but how do you determine what files to transpile? I assume there's some configuration to define something like “include ./src/**/*.ts,” right?

So basically, step 1 is to identify all the files that match that glob; and then step 2 is to transpile each of them. Since in step 1 you have the full list of all files that will be transpiled, in step 2 you can pass that list into transpileModule so it knows what files are on the list for every specifier it encounters, so it can rewrite those extensions and those extensions only. The option which enables this should be explicit that it's only rewriting extensions for files that TypeScript is transpiling, to ignore cases of import statements of non-local files or external libraries.

I understand the reluctance to fix something that isn't broken, in TypeScript maintainers' view, but browsers and Deno will never support CommonJS-style implicit extensions and it's not likely that Node will either (for ESM). If that's what TypeScript continues to insist upon, it will increasingly feel like a bug to users. Maybe that's not a “pot of gold,” but I think good UX is worth striving for.

@DanielRosenwasser
Copy link
Member Author

@GeoffreyBooth

Since in step 1 you have the full list of all files that will be transpiled

This is what we're saying is not the case. transpileModule shouldn't have to hit the disk or resolve at all. The model that most tools like bundlers, Babel, and others operate under is that files only have the local context, nothing more. transpileModule is a concrete implementation of a model that's prevalent in the ecosystem.

Forgive my ignorance, but how do you determine what files to transpile? I assume there's some configuration to define something like “include ./src/**/*.ts,” right?


The option which enables this should be explicit that it's only rewriting extensions for files that TypeScript is transpiling, to ignore cases of import statements of non-local files or external libraries.

include and files specify the entry points, through which we find the the transitive closure of files to be compiled via things like transitive imports. But trying to say "this flag explicitly doesn't work the way TypeScript works in other cases" is what makes Ryan's statement even more pertinent.

We added an option called exclude that filters what include picks up; with regularity someone shows up and literally insults our intelligence for making this setting not affect the totally unrelated process of module resolution.


browsers and Deno will never support CommonJS-style implicit extensions and it's not likely that Node will either (for ESM). If that's what TypeScript continues to insist upon

Again, nobody's insisting on this. You can add a .js to the end of your import paths and it will work!

@GeoffreyBooth
Copy link

You can add a .js to the end of your import paths and it will work!

For Node, but for IDEs? Is Code going to start seeing import './file.js' and look for file.ts there? And linters etc.? Even if the whole ecosystem adapts to this, it's bad UX to ask users to write paths to files that don't exist. That includes import './file' for non-CommonJS contexts.

I understand that changing the model of transpileModule, even if only when this option is enabled, is a painful change to make with ecosystem repercussions. But that seems like pain for the TypeScript development team and possibly authors of integrations like Babel's TypeScript plugin, but little if any pain for end users.

@DanielRosenwasser
Copy link
Member Author

DanielRosenwasser commented May 12, 2020

For Node, but for IDEs? Is Code going to start seeing import './file.js' and look for file.ts there?

Yes because TypeScript is literally the thing that powers the TS/JS IDE functionality in VS Code.

@DanielRosenwasser
Copy link
Member Author

DanielRosenwasser commented May 12, 2020

And if you have TypeScript editor functionality that isn't powered by...TypeScript, well, resolving that way is how TypeScript works so you'd have a bug if it wasn't implemented that way.

@MicahZoltu
Copy link
Contributor

MicahZoltu commented May 12, 2020

// <root>/apple.ts
export const apple = 'apple'

// <root>/index.ts
import { apple } from './apple'
console.log(apple)

Am I incorrect in assuming that when transpileModule is called for index.ts it will go to disk and find ./apple (which it will find as <root>/apple.ts) prior to emitting index.js? If that assumption is correct, then I think the argument being made is that if that specific resolution for apple, encountered while compiling index.ts, resolves to a TS file, then, and only then, would index.js be emitted as:

import { apple } from './apple.js'
console.log(apple)

If that assumption is incorrect, how does transpileModule manage to actually type check anything since the contents of ./apple are required for type checking index.ts?

@MicahZoltu
Copy link
Contributor

As a user, the scenario above is really the only time I want TS to rewrite imports for me. It is in this situation specifically that TS has more information than I do as the developer, and thus can more accurately figure out what the correct import URL should be. If I am importing something that resolves to a .d.ts it means that the extension of the runtime import is known or knowable to me at dev time, and I can correctly do import { banana } from './banana.mjs' or import { banana } from './banana.js or whatever.

The problem with writing import { apple } from './apple.js' when there is no JS on disk is that I don't yet know what the final extension on disk will be, because TS hasn't written it yet! Will TS write .mjs or .js or no extension or will this be run in ts-node where it will never write to disk and instead will import .ts? This is a question I cannot answer at dev time if I am using multiple tsconfig.json files and/or running inside ts-node or deno or whatever environment.

@timonson
Copy link

timonson commented May 12, 2020

huge pot of gold on the other side of the rainbow

@RyanCavanaugh the pot of gold is the possibility to download any TS module from any place in the world via URL without any dependency like npm, rollup and so on. Maybe even directly into the browser at one point.
Don't you have the vision for the near future where the compilation of TS happens in a background process in a 1/1000s? Well, I am not sure if this will ever be doable this fast, but even the possibility looks like gold to me!

@weswigham
Copy link
Member

weswigham commented May 12, 2020

Having individual file emit depend on disk/internet content would be a big barrier to that goal, not something that helps it in any way.

@timonson
Copy link

How @weswigham? You can cache content.

@weswigham
Copy link
Member

That's still miles worse than not needing to fetch the content at all. "Just cache it" does not magically make it free, it just amoritizes the cost over many similar requests.

Requiring dependency information to solve emit is just as bad as type directed emit, imo. And I can talk from experience here, as while we take the position that we don't rewrite import specifiers and always have, we do rewrite triple slash references (which are largely a legacy feature, which modules should never need to use) to referenced declaration files in declaration emit, and it is the biggest pain, and has resulted in hundreds of bug reports (many of them performance related!), and inspired features like types directives expressly designed to avoid it (which are mostly all that's used in DT nowadays), since it turns out there's absolutely no real world intuition as to what the compiler should (or should not) do to fix up a path, despite what anyone in this thread has said or dreamed up, since in the real world, output environments are actually hideously complicated. We have literally received conflicting bug reports asking for directly opposed behavior with respect to triple slash reference paths. There is no good behavior here. Attempting to edit paths at compile time should be avoided at all costs.

@timonson
Copy link

timonson commented May 12, 2020

I wished there was an emoticon expressing thank you so that I would not have to waste space in this thread thanking you @weswigham. So let me thank you for this great reply and the very good reasons (although my stupid intuition is still disagreeing with you) nevertheless in this way.

@Jack-Works
Copy link
Contributor

So what about my initial PR content? Don't smartly try to resolve the correct file extension, add it dumbly and tell the developer the rule of adding trailing extension. Developer should ensure the correctness by themselves

@DanielRosenwasser
Copy link
Member Author

@Jack-Works I think the overall sentiment is that always applying the extension change is likely to still have its own share of surprises which we're not convinced would be worth it:

This is the thing - a mode where you always apply a file extension change is wrong because it can't resolve.


What is a file extension, anyway? Is it unfathomable to see a repo with filenames parser.ts, parser.json.ts, and parser.js.ts ? What does an import of parser.js or parser.json mean in that setting? Is the answer so obvious that no one could ever be surprised? Can you design this in a way that adding a new file to disk never changes the output of "unrelated" files? How does this work in hosted scenarios where TS can't see what's on disk?

The current rule could not possibly be simpler, and cannot possibly break a working program. You'd have to convince us that there's a huge pot of gold on the other side of the rainbow to give that up.

@knightedcodemonkey
Copy link

knightedcodemonkey commented Jul 29, 2023

If specifier and file extensions are driving you bananas then try @knighted/specifier to change them however you like.

By the way, support of --module commonjs in combination with .mts file extensions is broken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Notes Notes from our design meetings
Projects
None yet
Development

No branches or pull requests