Xterm.js: Fit addon should not use private API

[Tyriar]

Follow up from https://github.com/xtermjs/xterm.js/pull/2164

[lanwen]

Don't know if related, but right now with the new api

this.fit = new FitAddon();
this.term.loadAddon(this.fit);

this.fit.fit() crashes with TypeError: Cannot read property 'dimensions' of undefined, trying to get dimensions on cols: Math.floor(l / e._renderService.dimensions.actualCellWidth)

with the debugger, I can see that actual container holding the dimensions is _renderCoordinator

So is that a bug and something wasn't updated in the addon? Or am I doing something wrong?

[Tyriar]

@lanwen that's happening because I didn't think through the versioning implications of the addons and we did some refactors so now the latest fit addon doesn't work with the latest release (only with master). From v4 onwards we'll be posting the compatible version of the addons to use
(step 7 in https://github.com/xtermjs/xterm.js/wiki/Releasing#release-notes).

Until you go onto v4 I'd recommend just sticking with the old applyAddon API

[jerch]

@Tyriar Do we need an API identifier on addons so the loading mechanism can deal with that? Imho for v3 to v4 this does not make much sense yet, but v4 might not be the last addon API?

Something like this:

// loader
...
switch (addon.API_VERSION) {
  case 4:
    v4Load(addon); // using v4 interface, to be removed at some point
    break;
  case 5:
    v5Load(addon); // using a newer interface
    break;
...

This might help during future addon API migrations - we could come up with a new version and keep supporting the older one for a couple of releases, so ppl can migrate their addons within that timeframe.

[Tyriar]

@jerch yeah we probably do need something like that eventually, most of these breakages would only be happening on major version changes (or big internal refactors if private API is used). This one's about layering refactors so maybe it's a good excuse to try add API and close out issues like this before v4 🤔

[mofux]

Another way to approach this is to define a peerDependency on xterm.js inside the package.json of the addon. This way npm would warn you if you were trying to load an addon that is not compatible with v4, or if you try to load an addon that depends on xterm.js v4 but only has xterm.js v3 as a peer dependency.

[Tyriar]

I tried the peerDependency route and it felt hacky working with it, especially during dev as we don't need the peer dep while working on the modules so we would get a bunch of warnings.

But I don't think a peer dependency would fix the major issue here which is internal breakages due to changes to private stuff, pretty much all the reports are this were the change from _renderCoordinator (v3) to _renderService (v4). The only way to support that if we wanted to would be to detect it like if ('_renderService' in term._core), and since I don't really want to do that I want thinking we could take the evergreen approach and just list compatible addons versions in each release and only really support the absolute latest one.