I think you should be able to export class here, similar to export class Terminal. That way we don't eed to deal with this CtorType stuff

Isn't that pulling types from internal sources into the public API? My idea here was to decouple that with minimal stub types, so linter / type inspection doesn't rely on internal stuff.

Isn't that pulling types from internal sources into the public API?

Everything in the API needs to be standalone, so we'd duplicate it there.

Also, we depend the other way for Terminal here to ensure our implementation matches the API:

So we could do the same for DisposableStore/Emitter/etc. by depending on xterm.d.ts from public/Terminal.ts again.

Maybe it's nicest to just pass in the whole xterm API object to some addons? I think we can do this without error:

import type * as XtermApi from '@xterm/xterm';
import type { Terminal, ITerminalAddon, IDisposable } from '@xterm/xterm';

That way it would be nicer from the embedder side:

new ProgressAddon(xterm)
new WebglAddon(xterm)
etc.

Looks better than this imo:

new ProgressAddon(AddonDisposable)
new WebglAddon(AddonDisposable)

Yes, thats a good idea. The dispoable + emitter ctors, if both are needed, already make this cumbersome and looking ugly. With the whole exports as one arguments it gets much nicer and easier to comprehend on caller side.
I even wonder if we should make that the first default argument on all addon ctors, this way ppl wont get it wrong on certain addons, just apply it on all (well thats a major API shift).

Well the full module export type interface stubbing felt kinda wrong. My next approach looks like that:

on addon impl side:

import { ..., ISharedExports } from '@xterm/xterm';

export class AddonXY implements ... {

  constructor(sharedExports: ISharedExports) {
    // do something with things exposed under sharedExports
  }

on embedding side:

import type { Terminal, sharedExports } from '@xterm/xterm';
import { AddonXY } from '@term/addon-xy';

const term = new Terminal(...);
const addonXY = new AddonXY(sharedExports);

In retrospect it's obvious this would be a breaking change, but that's fine and worth it considering the wins we get in bundle size.

About breaking change - as I wrote above, I wonder whether to make the xterm exports the first ctor argument for all addons, e.g.:

import * as XtermApi from '@xterm/xterm';
import { AddonXY } from '@term/addon-xy';

const term = XtermApi.Terminal(...);
const addonXY = new AddonXY(XtermApi);

Then an addon ctor is free to use the exported ctors there or to ignore that argument:

import type * as XtermApi from '@xterm/xterm';
class AddonXY extends ... {
  contructor(xterm: XtermApi, other_args) {
    // if addon needs an event:
    this._onWhatever = xterm.Emitter<Whatever>();
    ...
    // else: just ignore xterm argument
  }
}

Or if thats too "globalish" looking, we could also aggregate the extra exports under a shared API endpoint:

import { Terminal, shared } from '@xterm/xterm';
import { AddonXY } from '@term/addon-xy';

const term = new Terminal(...);
const addonXY = new AddonXY(shared);

And last but not least - we could also keep Terminal the only exported impl endpoint and put the shared stuff on the terminal class instead:

import { Terminal } from '@xterm/xterm';
import { AddonXY } from '@term/addon-xy';

const term = new Terminal(...);
const addonXY = new AddonXY(Terminal.shared);

The latter has a few advantages, like keeping stuff under the Terminal umbrella and automatically gaining access to those symbols even on a terminal instance.

Edit: Tbh the ctor argument idea raises in fact the question, why not to load addons this way in the first place with a terminal instance as first argument. Do you remember, why we have the loadAddon mechanics on the terminal the way it is implemented currently?

why not to load addons this way in the first place with a terminal instance as first argument

Terminal has always been the only thing exposed on the API, but it makes a lot of sense to pass in the full API when we start adding new things. This sort of breaking change is more impactful though as we can't just expect all addon ctors to have it as the first arg.

Do you remember, why we have the loadAddon mechanics on the terminal the way it is implemented currently?

Was a long time ago, but one of the big things we get is loadAddon lets the Terminal take ownership of it. So disposing of a terminal means the addon will be destroyed. That's pretty much all AddonManager does.

Yes it def. smells like quite the big API change would be needed, so idk exactly how to proceed. Maybe we should go back to conceptual structuring before inventing a square wheel here? Gonna try to do a write up of what we have currently vs. what could be done about it in #5283.

Posted long replies in #5283 (comment)