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

Discussion: Determine how to reinstate support for classic two-file components #760

Open
machty opened this issue Sep 23, 2024 · 0 comments
Assignees

Comments

@machty
Copy link
Contributor

machty commented Sep 23, 2024

Volar offers two variants for building modern type-checked language tooling:

  1. Language Server aka takeover mode (legacy) - the Language Server (e.g. Glint) spins up its own tsserver instances with Language-specific support (e.g. .gts files), and the user is encouraged to disable any built-in editor support for TypeScript files so that the Volarized LS (e.g. Glint) can/will handle all of .ts/.gts/.gjs/etc files (disabling vanilla TS is important otherwise you get duplicate diagnostic errors on .ts files). Glint also recommends this approach in the docs.

  2. TypeScript Server Plugin aka hybrid mode - this is the approach / direction taken by modern Volar: language tooling (diagnostics, auto-complete, etc) for .gts/.gjs files is provided via a TypeScript Server Plugin that runs within the editor's default/vanilla tsserver. A Language Server is still spun up to handle any non-type-specific tooling (TSPlugin + LS = "hybrid"). This approach has many benefits, including better interop with other TS plugins and avoiding the configuration awkwardness of disabling the default TS LS.

Two-File Components

Legacy LS takeover mode is basically already implemented in Glint 2 and offers excellent support for .gts/.gjs files, but unfortunately two-file components are not supported in Volar's takeover mode. Support for two-file components (whether Ember's classic component format or Angular's .ts + .html components or others) is only supported by Volar's TS Plugin API (specifically the getAssociatedScript API is only invoked when running in TS Plugin context).

How to proceed

Option 1: Push for Volar to add support for two-file components in legacy LS takeover

Johnson, I believe, has mentioned he's open to doing this, but we should make sure it's definitely necessary because I don't think anyone else is asking for it.

  • Pros
    • Defer the challenges/unknowns of TS-Plugin-izing until later, keep the V2 train moving until we've achieved true Glint 1 feature parity
  • Cons
    • Feels bad to push Volar to upstream support for something everyone else is moving away from; don't want to consume too much of their resources
    • The sooner we can get people to TS Plugin the better (for all the cons of legacy LS takeover mode described above)

Option 2: Migrate Glint to newer TS Plugin API

  • Pros
    • All the pros of TS Plugins described above
    • Alignment with present and future of Vue tooling; most Volar/Vue bugfixes will be in the TS Plugin realm going forward, better for us to hitch our wagon to TS Plugin
  • Cons
    • Volar has no testing API for TS Plugin-driven tooling
      • for LS there is a harness for spinning up a fake LS and running completions, diagnostic, etc commands against it and testing its response, and Glint heavily uses this testing API.
      • ideally we'd just be able to use this same testing API (or something close) and run parallel CI jobs, one in LS takeover mode and the other in TS Plugin mode, but this is not possible right now
      • Johnson says support for testing TS Plugin via Volar will be in Volar 2.5 (estimated mid-November)
      • I don't think it is practical / good use of my time to try and write something that directly uses the TS server harness linked above
      • So we have to:
        • wait for Volar to ship support for TS Plugin (halfway through Q4, and could very will be delayed), or
        • find some other way to validate TS Plugin behavior, or
          • maybe we write a handful of super slow full e2e integration tests that test basic loose mode functionality via TS Plugin
        • Punt on TS Plugin tests for now, verify manually, add tests later when Volar test support ships

Proposal

I think the best way forward is to:

  • Move to TS Plugin sooner rather than later
  • Implement a few slow end-to-end/acceptance test cases (test cases that spin up a new VSCode window, open a few files, and check diagnostics, etc) to test out TS Plugin mode
    • Ideally these are written in such a way that when Volar 2.5 ships with support for testing TS Plugin more directly, it won't be too difficult to "dissolve" these test cases into a faster integration test (like all of the other LS test cases)
  • Make it easy to swap between two modes when running Glint on an Ember project:
    • takeover mode: TS diagnostics come from Language Server; user is encouraged to disable vanilla TS
    • TS Plugin: TS diagnostics come from TS Plugin running within editor's vanilla TS
  • When Volar 2.5 ships with TS Plugin testability, we dissolve the slow acceptance test cases into faster integration tests
@machty machty self-assigned this Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant