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, 3/24/2023 #53500

Closed
DanielRosenwasser opened this issue Mar 24, 2023 · 6 comments
Closed

Design Meeting Notes, 3/24/2023 #53500

DanielRosenwasser opened this issue Mar 24, 2023 · 6 comments
Labels
Design Notes Notes from our design meetings

Comments

@DanielRosenwasser
Copy link
Member

--isolatedDeclarations

#53463

  • It is preferable to check against declaration files (.d.ts files) because there is no need to check against the implementation.
  • Wouldn't it be nice if you could emit those files without checking and just check against those?
  • --isolatedDeclarations ensures that your files could be emitted that way.
  • Most of our stuff is annotated, right?
    • For the majorish stuff?
    • Have to think about /** @internal */
  • Does this mode expect everything to be modules?
    • We don't think so.
  • Authors of esbuild and swc have both said they're interested in operating over this mode.
    • Are they interested in just declaration emit, or declaration bundling?
      • Need to clarify with others, but feels like both?
      • There are tools like tsup that just uses Rollup to do the same bundling logic over declaration files.
  • Would we use this on TypeScript itself?
    • Blocker is what our API is.
    • 2 stages:
      • emitDeclarationOnly to produce .d.ts files
      • Our bespoke bundler that merges it all.
  • How does this work for --build mode?
    • Currently we depend on if the .d.ts is older than the input files.
      • So check the JS files.
      • What if you're using noEmit? You need build-mode checking, but not emit.
      • .tsbuildinfo?
    • Can't meaningfully leverage this without parallelism
  • Is it expected that other tools would just do the transform, or would we?
    • Both, similar to transpileModule.
    • Big win for the community would be that others would understand this mode.
  • What about the DX around export const x = 10 - that's an error without a type.
    • Could be more lenient over time.
  • Unclear if people know what they're signing up for
    • What is the correct return type of every React component?
      • JSX.Element?
  • Declaration Maps?
    • Source-to-source translation, so it should be "free".
    • Yes, for us - what about everyone else?
    • Should really encourage implementers to support declaration maps in some way.
      • Really want go-to-definition to work well.
  • We're not sure if we want a separate walker - would rather use the existing declaration emitter.
  • Do we even need to support a mode here? Couldn't external tools do this?
    • Arguable that we have no work to support this mode - tools can already do declaration emit.
    • We're setting the base minimum, and giving you a heads up in the editor of what will and won't work.
      • Becomes weird if we make things more lax down the line.
@DanielRosenwasser DanielRosenwasser added the Design Notes Notes from our design meetings label Mar 24, 2023
@DanielRosenwasser
Copy link
Member Author

@dragomirtitian

@RyanCavanaugh
Copy link
Member

To expand/clarify where we are on isolatedDeclarations, here's my not-taken-live-notes read of the situation.

Overall, the scenario is obviously very compelling. Having this invariant enables a ton of very good optimizations that advanced build chains can take advantage of, and possibly opens up new doors for TS itself too.

The success of the isolatedModules flag in achieving its goals (1. have a confusing name 2. allow for fast correct transpilation) has shown that this can work very well.

Where it gets a lot hazier very quickly is that, as noted in the PR description, there are a lot of little dials and switches where a .d.ts-emitter (hereafter DTE) with a little bit of extra smarts could save a lot of small annoyances; this is confirmed by the Google feedback where just allowing a gentle inference of const n = 1 flavor of declarations can save a lot of headache.

We could start with a sort of zero-smarts isolatedDeclaration-reliant DTE in TypeScript and then add in smarts little by little as needed to keep it ergonomic while still ensuring the core invariant of not doing real semantic work was kept.

But the problem is, in all likelihood, the DTE that will be operating in these scenarios isn't ours. It's someone else's. While the semantics of emit under isolatedModules is fairly clear, already we have some divergence -- tools like esbuild can handle const enum, a thing which isolatedModules still says is illegal.

So if the ecosystem blooms here and there are three or four syntax-only DTEs, plus maybe one by us, which supported semantics set does isolatedDeclarations enforce? The lowest-common denominator? What if one with a lot of smarts gets a lot of traction, but a less-smart one is also popular? What if TS's DTE is the smartest but also the slowest? That's a mess; we don't want this flag to turn into twenty subflags.

But why would TS be the one enforcing those semantics in the first place? The DTE itself can tell if it's encountering a scenario it can't handle, and can presumably issue errors when it encounters them.. It'd be a little unfortunate to not have those errors directly integrated into TS for the purposes of language service reporting, but it's not the end of the world -- already we have things like lint rules which provide external syntactic errors. And the whole point of isolatedDeclarations is that these sort of checks should be in principle extremely fast to run, since they're not semantic.

It's a little confusing but it feels like the right outcome here, in terms of enabling a textured ecosystem of solutions, might be to simply do nothing and let whatever syntactic DTEs exist enforce their own rules. But we've also seen many times that solutions of the form "Let the community do X task which is syntactic and possibly very bespoke" have not really worked out in practice in a lot of cases. So we'd like to do something to encourage this pattern but it's not super clear what, yet. More thinking to come.

@DanielRosenwasser
Copy link
Member Author

There's a funny sort of interaction now that I reread the notes. I called out "What is the type of every React component? JSX.Element?" And it kind of runs into the weirdness @dragomirtitian noted about omitting /// <references. Without React's declaration files present in a compilation, the type returned here might be interpreted incorrectly (or further encourage the use of global types from React).

This is one of those problems that is maybe less technical and more about community guidelines. Something we should try to help change in the long-term.

@fatcerberus
Copy link

The success of the isolatedModules flag in achieving its goals (1. have a confusing name 2. allow for fast correct transpilation)

😆

@evanw
Copy link
Contributor

evanw commented Apr 1, 2023

Here's my perspective as the author of esbuild:

I believe this feature would be a lot more work for me (both initially and ongoing) if TypeScript ended up not adding isolatedDeclarations and instead left it up to individual tools. I expect that there would be a constant stream of issue reports for esbuild reporting both the divergence from TypeScript's full implementation and the incompatibility with the existing ecosystem of published npm type declarations. Hearing that isolatedDeclarations might not exist after all makes me not want to implement this feature.

Having isolatedDeclarations as an arbitrary line in the sand seems like it makes so much sense because it greatly reduces the burden on both tool authors and on users. Tool authors can use it to guarantee support for an easy-to-communicate subset of type declarations without lots of time-intensive testing of popular npm packages and/or maintenance overhead from a constant stream of incompatibility reports. Type declaration authors can use it to guarantee support for current (and future!) non-tsc tools without lots of time-intensive testing and/or ongoing work as new tools are written (or for less actively-maintained packages, likely just general frustration at things not working well across tools).

I get the point about cases like const n = 1 but it feels like there's strong value in isolatedDeclarations being a real thing. So if that's a concern, I'd say maybe include a few very simple type inference rules that are trivial to implement syntactically to make this a little more ergonomic, then stop there and lock in what isolatedDeclarations means.

@robpalme
Copy link

robpalme commented Apr 3, 2023

Thanks, Evan. That matches my understanding of the landscape too. There is a core ecosystem coordination problem to solve, where individual participants would be taking a risk if they tried to implement this feature alone. Whereas if we can iterate together towards an acceptable DX, the ideal solution is for TypeScript to act as the reference for the DTE specification and provide conformance tests. That is the easiest path towards adoption.


Here's some responses to other points raised earlier. Let's reuse Ryan's abbreviation DTE to mean a *.d.ts emitter.

First-class TS errors vs external errors

Do we even need to support a mode here? Couldn't external tools do this?

The DTE itself can tell if it's encountering a scenario it can't handle, and can presumably issue errors when it encounters them

Type-driven enforcement of the rules by the TS checker can provide better DX by freeing up per-file DTEs to be more optimistic about the kind of declarations they emit. For example consider the following snippet:

let o = new SomeClass()

In most cases the above can be safely transformed to the following declaration:

let o: SomeClass;

But without the type information about SomeClass a DTE can’t know this for sure. TS can enforce rules that make the above safe. For example requiring that SomeClass is indeed a class, or at least that the instance name is the same as the constructor name, and require that any type parameters are explicitly specified. An external tool performing standalone per-file analysis cannot be as good as TS in this respect.

Additionally, TS having built-in error generation also means we can have high-quality type-driven quick-fixes to resolve these errors on the user's behalf. This seems important for DX.

Bundling

Are they interested in just declaration emit, or declaration bundling?

swc is interested in both according to Donny. However declaration bundling seems fully orthogonal to Isolated Declarations because any tool is already free to bundle today using the results of tsc's declaration emit.

Parallelism

Can't meaningfully leverage this without parallelism

Right. Our primary motivation for this feature is to enable project-level parallelism. In our case we use monorepo tooling that spins up multiple processes that each run tsc on separate projects.

tsc --build itself could definitely leverage parallelism in future. Probably that would be best as an independent follow-on project that is not part of the initial scope. There will be many innovations that can build on the foundations of Isolated Declarations 😉

Small Correction

What about the DX around export const x = 10 - that's an error without a type

export const x = 10 is not an error because declaration emit preserves the primitive assignment in declarations for const declarations.

export let x = 10 is currently an error, but can be improved by DX Improvement D2.

Declaration Maps

Should really encourage implementers to support declaration maps in some way

Agreed. We will add that to the emitter and to the conformance tests to ensure all ecosystem DTE implementations do a good job of this.

One vs Two Emitters

We're not sure if we want a separate walker - would rather use the existing declaration emitter.

The new emitter was created separately only for ease of iteration and prototyping. Let's keep it separate for now whilst we agree on the high-level feature decision points. The final location is a TS decision and we will be very happy to migrate it into the existing emitter when that is desired. The existing emitter only needs two additional components to make that work: the non-type-checked emit resolver and possibly a simplified binder.

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

5 participants