Xterm.js: Improved Layering and enabling TS strict

Created on 10 Jun 2018  ยท  15Comments  ยท  Source: xtermjs/xterm.js

Background

While in Prague, @mofux and I got together to discuss a better architecture for xterm.js that could lead to things like pulling all DOM access away from the core, moving parts of xterm.js into a web worker, and swapping out the frontend completely. Notes from this discussion were captured in https://github.com/xtermjs/xterm.js/issues/1506

Proposal

I propose we split the codebase into distinct parts with very specific roles. This could be done all in the VS Code repo and still result in only a single xterm module being published, while also not obstructing a future where we may want to allow swapping parts out.

Here is the breakdown:

  • src/ - Contains all source code as today

    • base/ - Contains interfaces for core and frontend to communicate with each other, as well as code that is used between both of them

    • core/ - This runs in both node.js and browser runtimes. This means far better and easier testing since everything is just node.js without worrying about the DOM

    • ui/ - This code runs only in a browser, anything in xterm.js that interacts with the DOM today will move to here. That includes things like mouse wheel handling (and translating), composition helper, textarea, etc.

    • public/ - This provides an implementation of the current day xterm.js API.

Dependencies between this folders are strictly enforced:

|------------------|
|       base       |
|------------------|
    โ†‘       โ†‘    โ†‘
|------| |----|  |
| core | | ui |  |
|------| |----|  |
    โ†‘       โ†‘    |
|------------------|
|      public      |
|------------------|

Notice that ui does not depend on core, any dependencies between core and ui are pulled into interfaces and placed into base.

Example file structure

This tree will give you an idea of what lives where:

src/
  base/
    Types.ts (maybe this should be .d.ts?)
    Clone.ts
  core/
    Types.ts
    Core.ts
    components/
      Buffer.ts
      BufferSet.ts
      Parser.ts
  ui/
    Types.ts
    UserInterface.ts
    components/
      Accessibility.ts
      Composition.ts
  public/
    xterm.d.ts
    Terminal.ts

Composite projects

In TypeScript 3 we will get the ability to build parts of projects independently and depend on them. This would lead to improved build times but also we get the layering enforcement for free as the tsconfig.json for core for example will not contain the dom APIs, so any use of them will fail compilation.

Absolute imports

We may be able to use absolute imports when these changes happen, whenever I try to do it I run into issues with importing a file from the same folder as an absolute import. See https://github.com/xtermjs/xterm.js/issues/1314

Plan

This is quite a large shift from the way things are done now and therefore will take a fair bit of time to execute. Luckily it can be done in parts and many in parallel. It may be best to move to this first:

|------------------|
|       base       |
|------------------|
    โ†‘            โ†‘
|------|         |
| core |         |
|------|         |
    โ†‘            |
|------------------|
|      public      |
|------------------|

And slowly separate core from ui.

typdebt

All 15 comments

See this PR for more specifics https://github.com/xtermjs/xterm.js/pull/1508

+1 for getting DOM related stuff strictly separated, it makes it possible to use core parts for offscreen things like an expect lib etc. which also implies that the core parts should be runnable in a pure nodejs env (without any browser engine stuff). This would also help to get it moved into a webworker later on (although this might need further glue code for interaction).

I wonder at what level of integration you intend to build the later API - will this become framework like where people can pick & alter stuff and just mount all together or will it still form the current more "monolithic" API? If we go with the framework idea and expose most of the internals we may want to deliver a default "easy to go" preconfigured setup as well (might end up delivering framework parts and a "monolithic" API, so not sure what works best).

To illustrate the framework/component idea further, take some buffering base class (no DOM) and add the following:

  1. a parser (no DOM)
  2. an escape code input handler (no DOM)
  3. optional: user input handler (keyboard + mouse, most likely DOM related)
  4. optional: an output renderer (most likely DOM related)

1 + 2 would form an offscreen terminal capable to deal with scripts/apps expecting a terminal without any interaction, with 3 added it would also be able to handle interaction. With 4 it would form a complete terminal with output representation.

If we go with the framework idea and expose most of the internals we may want to deliver a default "easy to go" preconfigured setup as well (might end up delivering framework parts and a "monolithic" API, so not sure what works best).

Less API is always better as it means more flexibility for us, having said that we definitely need to add a bunch of buffer APIs as addons rely on them. We'll see what works, this model gives us the flexibility of doing so in the future, my thinking was we provide the public, and maybe @mofux maintains a monaco-xterm in the future?

which also implies that the core parts should be runnable in a pure nodejs env (without any browser engine stuff)

This is the major win for me, not needing to worry about DOM and being able to do end-to-end tests of the core will be very valuable.

Nice seeing things getting into shape, I like the proposed design!

@jerch
I'd expect that core would provide methods for dispatching keyboard and mouse events that would take our own IMouseEvent and IKeyboardEvent.

IKeyboardEvent would be a simplified version of a DOM KeyboardEvent.
IMouseEvent would be a simplified version of a DOM MouseEvent, but with the mouse coordinates already translated to a row / column system.

@Tyriar
Took me a while getting used to the folder naming, but if I got it right, the folder names could also translate to:
base --> common or shared
core --> backend
ui --> frontend

I guess it's because the words base and core are synonyms for each other in my mind ๐Ÿ˜…Maybe changing base to common or shared would make the purpose clearer?

It's a really cool idea. If this will be really implemented you can easy change render to another one. Cloud 9 terminal for example it's a control sequence parser(seems old xterm) + ice editor(like render). If you really do good xterm.js splitting you can easy, for example, use Monaco editor to render terminal.

I guess it's because the words base and core are synonyms for each other in my mind ๐Ÿ˜…Maybe changing base to common or shared would make the purpose clearer?

Good idea, I stayed away from frontend/backend because it's not clear what the capitalization should be. Also I like core/ui as it's very clear what that means imo. So let's go with common instead of base then? That's the name used in both VS Code and chromium for similar concepts.

@Tyriar Yes, renaming base to common would fully satisfy my brain ๐Ÿ˜…

To be clear, there's a little missing information in this diagram:

|------------------|
|   base/common    |
|------------------|
    โ†‘       โ†‘    โ†‘
|------| |----|  |
| core | | ui |  |
|------| |----|  |
    โ†‘       โ†‘    |
|------------------|
|      public      |
|------------------|

ui does not depend on core, but it does own an implementation of ICore which lives inside common/ and operate on it directly. This is not the case the other direction; core will fire events that are listened to by ui, core does not own a UserInterface (ie. IUserInterface lives inside ui/, not common/).

export class Core {
    public f() {
        // Fire event on Core
        this._parser.on('refresh', d => this.emit('refresh', d));
    }
}
export class UserInterface {
    public f() {
        // Core communicating with UserInterface
        this._core.on('refresh', d => renderer.refresh(d));
    }

    public x() {
        // An example of direct communication to Core inside ui/
        this._core.send(this._arrowSequences());
    }
}

Here's what I see the implementation of public/Terminal looking like:

import { ICore } from '../common/Types';
import { Core } from '../core/Core';
import { IUserInteface } from '../ui/Types';
import { UserInteface } from '../ui/UserInterface';

export class Terminal {
    private _core: ICore;
    private _userInteface: IUserInterface;

    constructor(opt) {
        this._core = new Core(opt);
    }

    public open(element: HTMLElement) {
        this._userInterface = new UserInterface(this._core, element);
    }
}

Just went through all the files and I think these are the folders we want them in:

typings/
  xterm.d.ts (maybe live in src/public eventually?)
src/
  common/
    CircularList.ts
    Clone.ts
    EscapeSequences.ts
    EventEmitter.ts
    Lifecycle.ts
    Platform.ts (previously Browser.ts, even though this touches web APIs it works across platforms)
    TestUtils.test.ts
  core/
    Buffer.ts
    BufferSet.ts
    Charsets.ts
    CharWidth.ts
    Core.ts (previously parts of src/Terminal.ts)
    EscapeSequenceParser.ts
    InputHandler.ts
    Keyboard.ts
    Mouse.ts
    TestUtils.test.ts
    Types.ts (FLAGS from renderer/Types.ts should move here)
  ui/
    renderer/ (entire folder should live here)
      CharAtlasGenerator.ts (previously shared/atlas)
    mouse/
      AltClickHandler.ts (previously handlers/AltClickHandler.ts, should eventually live in an addon)
      Linkifier.ts
      MouseHelper.ts
      MouseZoneManager.ts
      SelectionManager.ts
      SelectionModel.ts
    AccessibilityManager.ts
    CharMeasure.ts
    Clipboard.ts
    CompositionHelper.ts
    Lifecycle.ts
    RenderDebouncer.ts
    ScreenDprMonitor.ts
    Strings.ts
    TestUtils.test.ts
    Types.ts
    UserInterface.ts (previously parts of src/Terminal.ts)
    Viewport.ts
  public/
    Terminal.ts (src/Terminal.ts should eventually be moved completely into this)

Also to clarify this as I confused myself just now, you may ask why not something like this:

|------------------|
|       core       |
|------------------|
  โ†‘           โ†‘  
  |       |--------|
  |       |   ui   |
  |       |--------|
  |           |   
|------------------|
|      public      |
|------------------|

The reason is because the eventual goal is to allow core to run in a web worker. That's the sole reason this common module exists. In such a world, public/Terminal.ts will not actually be used but another implementation will be done that enables communication between core and ui. This is what that would look like:

|------------------|                          |------------------|
|   base/common    |                          |   base/common    |
|------------------|                          |------------------|
    โ†‘           โ†‘                                 โ†‘           โ†‘
|------|        |                             |------|        |
| core |        |                             |  ui  |        |
|------|        |                             |------|        |
    โ†‘           |                                 โ†‘           |
|------------------| ------postMessage------> |------------------|
|    coreworker    |                          |     uiworker     |
|------------------| <--worker.postMessage--- |------------------|

That's perfect, structuring it this way is definitely the way to go IMHO ๐Ÿ‘. It also supports the ideas raised in #1515 very well.

After spending some time actually moving components over to this new architecture I have some changes I think we should make.

Merge core into common

Primarily the difference between core and common is confusing and probably could be classified as over engineered. The initial thinking was that ui would depend on only a small portion of common and definitely not touch core, but after revisiting it I don't think that's particularly useful.

This will also help with naming confusions as some of us have gotten into the habit of calling this repo "core".

In https://github.com/xtermjs/xterm.js/issues/1507#issuecomment-402304660 I call out that one of the reasons for common existing is so that we can have core and ui live in different threads/processes and communicate back and forth, for example in a web worker or running core on a separate machine. For the remote case it complicates things a lot in order to just move where the latency sits, both have downsides. For web workers, we would definitely want a portion of ui to run inside the web worker as one of the primary reasons for this direction imo is to move rendering code there via HTMLCanvasElement.transferControlToOffscreen, for example maybe with the new renderer API those render instructions can just be sent to a web worker which host the actual renderer. Regardless, we can think more about that problem when we get to it.

Rename ui to browser

If core is merged into common I think it makes a lot of sense to just align here and say common is the code that's common between node.js and the browser, and the browser module uses browser APIs. Basically I think it's a clearer way of defining it.

Services and dependency inversion

Trying to untangle the mess that is Terminal is quite challenging due to circular dependencies being everywhere (#1627). To get around this problem in a nice way we should define some services which basically pull parts out of Terminal, Buffer, etc. into services that are the source of truth for that area. For example I just pushed in an OptionsService in #2171 which nicely pulls all options-related things into a separate file that can be provided to components that need to interact with the terminal options.

We could create a Services.d.ts file in common and later browser which hold the interfaces for all the services these.

  • src/

    • common/

    • services/



      • OptionsService.ts


      • Services.d.ts



    • browser/

    • services/



      • SelectionService.ts


      • Services.d.ts



    • public/

Eventually we should also change Types to a .d.ts, that just means they won't actually produce any code, just interfaces. But one problem at a time...

Module dependency graph

With these changes the new dependency graph would look like this:

|--------------|
|    common    |
|--------------|
     โ†‘       โ†‘
|---------|  |
| browser |  |
|---------|  |
     โ†‘       |
|--------------|
|    public    |
|--------------|

@Tyriar Good thinking - lets not over complicate the repo structure. In particular I like the service idea to delegate and structure access to internals (as I really worry that a more direct access to internals that was requested several times will break at hard to track down ends).
Also I think we should not put to much effort into solving a problem, thats not yet at hand - webworkers.

Since API tests use node, I've moved them out to /test in https://github.com/xtermjs/xterm.js/pull/2203 and enabled strict

Some things to do before this is checked off:

  • [ ] Go through constructors to make sure things weren't left behind that are now in services (such as scrollToBottom, handler, etc.)
  • [ ] Verify no bad TODOs are slipping into code base
Was this page helpful?
0 / 5 - 0 ratings

Related issues

tandatle picture tandatle  ยท  3Comments

7PH picture 7PH  ยท  4Comments

Tyriar picture Tyriar  ยท  4Comments

johnpoth picture johnpoth  ยท  3Comments

Tyriar picture Tyriar  ยท  4Comments