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

Need a API to get the loaded Addons #2821

Closed
ScarlettKK opened this issue Apr 8, 2020 · 3 comments
Closed

Need a API to get the loaded Addons #2821

ScarlettKK opened this issue Apr 8, 2020 · 3 comments

Comments

@ScarlettKK
Copy link

ScarlettKK commented Apr 8, 2020

As is known, if we need to load some addons for terminal, we should use the following code:

import { Terminal } from 'xterm';
import { FitAddon } from 'xterm-addon-fit';

const terminal = new Terminal();
const fitAddon = new FitAddon();
terminal.loadAddon(fitAddon);
terminal.open(containerElement);
fitAddon.fit();

That means we still need the variable fitAddon if we want to execute the fit() function.

But this is inconvenient when the project is using modularization, we need to export more variables.

Since the terminal has already loaded the addon, can we set a getAddon API for terminal? The using will like this:

// terminal.js
import { Terminal } from 'xterm';
import { FitAddon } from 'xterm-addon-fit';

const terminal = new Terminal();
const fitAddon = new FitAddon();
terminal.loadAddon(fitAddon);
terminal.open(containerElement);

export default terminal;

// index.js
import terminal from './terminal';

const fitAddon = terminal.getAddon('fit');
fitAddon.fit();
@ScarlettKK ScarlettKK changed the title Need a API Need a API to get the loaded Addons Apr 8, 2020
@Tyriar
Copy link
Member

Tyriar commented Apr 8, 2020

I'm not really a fan of doing this for these reasons:

  • We don't disallow loading an addon twice
  • It's not typed well and there is no way I'm aware of that fitAddon = terminal.getAddon('fit') would be typed implicitly
  • You can just export an addons array or fitAddon directly

@ScarlettKK
Copy link
Author

Then how about using fitAddon = terminal.getFitAddon()?

We don't disallow loading an addon twice

The user can get the new addon again.

@Tyriar
Copy link
Member

Tyriar commented Apr 11, 2020

@ScarlettKK that wouldn't work either because we can't reference the name of addons in the core API (xterm.d.ts). This was considered when redesigning the types but trying to come up with a way to get addons back really complicated things and was a bit weird as well since you needed to pass the ctor into getAddon in order to get the instance of it back, see #1128 (comment)

I don't think we should do this because of how seemingly complicated this is to do right and there's a very simple workaround on the embedder side (just keep the reference to the addon object).

@Tyriar Tyriar closed this as completed Apr 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants