Design Meeting Notes, 2/24/2023

[DanielRosenwasser]

Libraries Patching TypeScript's API

https://gist.github.com/jakebailey/3e73c7aab4da2044a95121467aa3b4e9

• In TypeScript 5.0, libraries can no longer override properties of our API.
• Language service plugins
  • typescript-plugin-css-modules
    • Can no longer patch createLanguageServiceSourceFile and updateLanguageServiceSourceFile
    • Make ones up with CSS.
    • Should be proxying the language service host
  • vuejs/language-tools
    • Same deal with resolveModuleName
      • The new 5.0 resolution modes are better fits.
        • --moduleResolution bundler
        • --allowArbitraryExtensions
          • Use .d.vue.ts files.
    • There's really no host-level function they can override? Surprising.
      • Seems like no?
      • But resolveModuleName takes a ModuleResolutionHost - is there a way to control that?
        • Seems fairly file-system-oriented.
      • Actually some hosts do have a resolveModuleNames
        • proxyPluginHost should set host.resolveModuleNameLiterals.
    • Still, using the new resolution modes is recommended, but if they're determined to support node16/nodenext (they shouldn't, we don't believe it's actually the the right thing to use), patching resolveModuleNameLiterals is the right thing.
    • Meta: they recommend most users use this instead of our language server: https://vuejs.org/guide/typescript/overview.html#volar-takeover-mode
• Supporting Transformers
  • ttypescript
    • createProgram
  • ts-patch
    • createProgram, lots of other top-level stuff to enable caching.
  • The core idea is that these packages don't want to support all of the tsc command line and whatnot.
  • Is executeCommandLine public?
    • Yes. (later: no)
    • Then they should be using that.
    • Ehh, it's not exported.
      • Only internally exported for testing scenarios.
    • Also there's no emit hook. Can't plug transforms in today.
    • program.emit is overridable from executeCommandLine
      • But that happens after emit occurs.
      • But you can set noEmit and then force emit.
      • But then you don't get declaration file diagnostics.
  • Come back to these.
• Stencil - has its own compiler
  • Sets ts.sys
  • We have an /* @internal */ function setSys that they can use.
  • Also use resolveModuleName - they should be passing this into the CompilerHost they create
  • So actually, most of this should be on the CompilerHost
  • There are some functions they patch which we missed.
• Heft
  • Build system that wraps TypeScript.
  • readJson - only reason for this is caching.
  • emitFiles - patched so that they can support multiple emit targets.
  • Possible idea: you have the CompilerHost, just patch it every time and re-run emit.
    • Tried, but was unsuccessful.
• Yarn
  • Still works! Patched in a different way, maintained in a fork.
  • Would like to make this easier, but not in near-term range.

Comparing Wrapper Objects

• 1 is a number
• Number(1) is a number
• new Number(1) is a Number - a wrapper object.
• All numeric and relational operations turn the wrapper objects back into their primitive equivalents.
• === is suspect though.
• In 5.0, number < Number is disallowed
• We have a change that makes relational comparison consider valueOf (Stricter relational comparisons checking valueOf #52807) - which re-allows number < Number!
• We don't feel like it's a huge problem to error on these - would prefer people not use these types.
• People shouldn't be referencing these anyway - default typescript-eslint rule is ban-types which stops these.
  • So not unbreaking number < Number in 5.0 - it's too late in the game regardless of what happens in 5.1.
• Aside: probably should not perform these in the relationship checks, should liken these to the NaN checks.

[uasan]

The new 5.0 resolution modes are better fits.
--moduleResolution bundler
--allowArbitraryExtensions
Use .d.vue.ts files.

To integrate modules that are not .js, .jsx, .json, there are 2 options to create real-time d.ts files or hack system TS methods.

We chose the second option, we overridden the hostBuilder.resolveModuleNames method for file observe of custom extensions (.css, .svg) and ts.createSourceFile (I know this is a terrible hack) in this method for custom extensions we use the corresponding parsers and based on their AST we generate and return text d.ts.

Creating d.ts files is much slower and triggers a lot of processes, git plugins, eslint, etc...

Options --moduleResolution bundler and --allowArbitraryExtensions, do require d.ts files, but we need a legal method in which we can get the contents of custom files and return d.ts text without creating d.ts files, perhaps this method already exists, but I don’t know?

[johnsoncodehk]

Vue also patched supportedTSExtensions, supportedJSExtensions, allSupportedExtensions, createProgram via fs.readFileSync for tsc. (I think TS 5.0 can't prevent this. 👀)

https://github.com/vuejs/language-tools/blob/04d84f604e0fbf025f76f58a46c75b59bc1ca57e/packages/vue-tsc/bin/vue-tsc.js#L13-L18

[jakebailey]

For the LS plugin, you should be able to do everything you need by providing host methods.

I didn't find that vue-tsc, and that's really unfortunate. But, that's not a runtime patch, kinda like some of the other ones, and is already fragile as it is...

[uasan]

The host does not have a method that is responsible for returning the synthetic content of files with a custom extension, which is why you have to do these dirty hacks.

If d.ts files are created synthetically in runtime, then it is more efficient not to create these files on the file system, you need to add a method to the host that can be overridden, which will return the generated content in the d.ts syntex for files with custom extensions.

[jakebailey]

The host does not have a method that is responsible for returning the synthetic content of files with a custom extension, which is why you have to do these dirty hacks.

I'm a little confused; is readFile not enough?

[jakebailey]

In any case, this issue is not the right place for this sort of discussion; there's an issue for the css plugin (#52847), and I think it'd be best if there were a distinct issue for the APIs that you believe are missing too, so we can either track the addition, or commentary on a safer way to do the same thing.

[uasan]

I'm a little confused; is readFile not enough?

No readFile, only called for files that exist on the filesystem, we don't create files on the filesystem.

Ok, I'll open a new issue, thanks.

[jakebailey]

Sorry, I should have expanded that more; readFile + the other FS related methods, like readDirectory and such too.

[uasan]

issue for the APIs that you believe are missing too, so we can either track the addition, or commentary on a safer way to do the same thing.

Done
#52983