Design Meeting Notes, 11/15/2019

[DanielRosenwasser]

Syntax for Type Re-exporting

#34750

This is a story about 2 camps of people that are dissatisfied about our import/export emit for opposite reasons!

Camp 1

e.g. Angular users relying on side-effects, but we perform import elision!

import { Component } from "./component";

export class Service {
    constructor(component: Component) {

    }
}

declare var ng: any;
ng.registerService(Service)

• We can suggest import "./component"; - is that problematic?
  • Yes, original form has different semantics, why would you make users think about this?

Camp 2

e.g. Babel users, using isolatedModules, who might be re-exporting just types, but have no way of signaling that the export * has emit.

export { SomeType } from "./mod";

Webpack ≤4.x used to warn on this, Webpack 5 will actually throw.

• A while ago, we had a proposal for import type, much like what Flow has today.

  import { type X } from "./a";
  import type { X } from "./a";

  • Okay, seems cool. What about this syntax?

    import type Class, { Type, Value } from "./a";

  • Uh, which is a type, which is a value?

    • If we did this one, maybe we'd either do "everything is a type" or "everything needs a type prefix"

  • Part of the motivation also is just to t

  • In a clean room implementation, maybe we would've picked a syntax like type import or typeonly import.

• Bikeshedding aside, is this a feature we wanna do?

  • Would we want to error on existing code?
    • Tempting. Maybe a in strict mode?
  • Not clear what you want to error on - who is benefitting from an error?
    • --removeUnusedImports
    • Then change the default down the line to false.

• What does each camp of users do?

  • Babel/transpileModule users just write import type to get around existing errors, Angular users turn off import elision.
  • Things are getting hairy though, hard to reason about.
  • Conclusion: Ryan to work with Andrew to provide a write-up.

Performance

• Start-up Feedack
• Navigation Feedback
  • Investigate Delayed Go-to-Definition Behavior in Editors #34783

Preventing Misconfiguration

#35121

Cached Project Opens

#35113

Lazy Binding

#35120

Explorative Operations

#35114

Grace Period for Unloading Entire Projects

#35115

---

Notes from @uniqueiniquity

Performance Issues

Exclude globs are wrong

• Users often end up with weird misconfiguration issue, specifically in monorepos
• End up with config file at top level with incorrect exclude glob
• For example
  • We implicitly dive into top-level node_modules, but not subdirectory node_modules
• In trying to fix this, users end up making the same specification manually
• Two proposed fixes:
  • Harden our default to handle this
    • Seems like a good idea, easy win
  • Specify "implicit excludes" which everyone gets, and there's a way to turn that off but not modify
    • Saving people from themselves
    • Less clearly a good idea
• Make sure the first time excludes are mentioned in documentation, it does the right thing
• "You don't know what you don't know"
  • You know your program is slow, but you don't know it's the exclude glob, so you don't know the easy fix
• Should we warn you if we ingest "large amounts" of node_modules?
  • Where's the cutoff?
  • TL;DR we're doing a lot of work and finding nothing.
• Specific slow down is root file name search
• Would it ever be correct to have source files in the node_modules?
  • Daniel says no
• How would you opt-in for scanning?
  • Not quite clear
• One thing we could consider is a switch you could turn on that says "analyze performance" or "give me performance hints"
  • We could put in a bunch of hints so people could better help themselves
  • Skepticism bc users won't take the time to try it and would rather complain
  • But if you give the community the tools they should use it
  • Should offer a "typical" percentage?
• Hesitance about fuzzy metric to start warning people by default
• Is there room for the editor here?
  • When would you ask for these diagnostics? Should they be suggestions in the editor?
• Fix the default, yeesh

Navigation Issues

Loading projects faster

• Navigating can jump into a previously unloaded project, which can cause a sudden, costly, perf hit
• Today, we fix this cost by doing incremental mode (.tsbuildinfo)
• What if we could leverage tsbuildinfo files or similar that could be leveraged by the server
• Today, doesn't contain resolution information, but rather just a reference tree
• Server/tsc-watch is responsible for incrementally watching directories to incrementally update
• Saving information on disk doesn't really help because you don't know what has changed
• Is there any room here for tsbuildinfo to help the server?
  • Not clear
  • You still have to do a parse, and tsbuildinfo can't help you there

Avoiding loading projects in preview windows

• Is there room here to be able to "peek" into a file in another project without loading the whole containing project?
  • This happens in go-to-def, or even editor peek
• You really just want colorization and outlining, but probably nothing else
  • Until you hover… that's the stumbling block
• Editors could tell the server that they're in a "temporary buffer"
  • Preview windows seem like the easier case
• Different classes of hover
  • How do we measure intent?
  • People don't actually use the keyboard shortcut for quickinfo/sighelp
• What's an experience for a peek into an unloaded project?
  • Bar to click here to fully load?
  • "Quick info not available in…"
• This happens when a file is in your current project, but its default project is not your current project
  • Project references scenario
  • E.g. types.ts in compiler vs transitively available through services
• Taking this conversation offline

Grace period for unloading projects

• Working in two different projects, accidentally unload one by closing tabs and then have to reload
• Should we have heuristics around "waiting a while" before unloading a project?
• This seems secondary to the smarter loading of the detached file, so let's investigate that more first

Lazy binding

• Global scope is a necessary consideration
• Today on project load, we bind all the files (specifically when the checker is created)
• Proposal is to only bind things that you dive into
  • Use parse information to know what needs to be bound up front (e.g. UMD globals)
  • When you resolve a module, then you bind the target file
• What are the edge cases?
• Perf tests pass
• What's the benefit?
  • Not sure yet, parse is the more expensive part of the process
  • Intended for quick info, go-to-def
  • Prevents excessive binding of .d.ts files in node_modules
• Does incremental parser need binding?
• Issues in prototype
  • What are the implicit expectations that files are bound?
  • You can get GC-like pauses when you first pull on a file
    • Creates warm-up period like checking, but without the need to throw away whole-project state on changes
  • Failed all the fourslash tests 😊

[jkillian]

One thing we could consider is a switch you could turn on that says "analyze performance" or "give me performance hints"

I would love some sort of feature like this! I've been noticing recently that our VSCode TypeScript experience has felt somewhat slow and have been wanting to figure out why. I found the --extendedDiagnostics flag, which is great, but pretty hard to convert in to concrete actions. I was actually googling around for "How to debug TypeScript performance" the other day, but didn't find too many useful resources.

A feature with easy to grok performance diagnostics and hints would be really cool, could perhaps include things like extraneous included files (like discussed above), perhaps some sort of list of the slowest/most-troublesome types in the codebase (don't know how realistic this is or not), etc.