Repository: pyscript/pyscript
File: pyscriptjs/src/plugins/pyterminal.ts (TypeScript)

Code before:
        outElem: HTMLDivElement;
        moduleResolved: boolean;
        xtermReadyPromise: Promise<TerminalType>;
        xterm: TerminalType;
        cachedStdOut: Array<string>;
        cachedStdErr: Array<string>;

        constructor() {
            super();
            this.moduleResolved = false;
            this.cachedStdOut = [];
            this.cachedStdErr = [];

            //Required to make xterm appear properly
            this.style.width = '100%';
            this.style.height = '100%';
        }

        async connectedCallback() {
            this.outElem = document.createElement('div');
            //this.outElem.className = 'py-terminal';
            this.appendChild(this.outElem);

            this.setupPosition(app);

            this.xtermReadyPromise = this.setupXterm();
            await this.xtermReadyPromise;
        }

        async setupXterm() {
            // eslint-disable-next-line
            // @ts-ignore
            if (globalThis.Terminal == undefined) {
                //xterm module proper
                //eslint-disable-next-line
                //@ts-ignore
                await import('https://cdn.jsdelivr.net/npm/xterm@5.1.0/lib/xterm.js');

                const cssTag = document.createElement('link');
                cssTag.type = 'text/css';
                cssTag.rel = 'stylesheet';
                cssTag.href = 'https://cdn.jsdelivr.net/npm/xterm@5.1.0/css/xterm.css';
                document.head.appendChild(cssTag);

                //Create xterm, add addons
                this.xterm = new Terminal({ screenReaderMode: true, cols: 80 });

                // xterm must only 'open' into a visible DOM element
                // If terminal is still hidden, open during first write
                if (!this.autoShowOnNextLine) this.xterm.open(this);


Review comment (refactor):
if this is how users should await the terminal I feel like `setupXterm` shouldn't be a public method, rather an external private funciton/utility to `.call(this)` or at least a `_setupXterm` prefixed method because it's otherwise exposed as API nobody should use when `connectedCallback` has not happened ... can we put at least a `@private` hint to the method or make it more obvious it's an internal implementation detail?

Code after:
    /** The <py-terminal> custom element, which automatically register a stdio
     *  listener to capture and display stdout/stderr
     */
    class PyTerminalXterm extends PyTerminalBaseClass {
        outElem: HTMLDivElement;
        _moduleResolved: boolean;
        xtermReady: Promise<TerminalType>;
        xterm: TerminalType;
        cachedStdOut: Array<string>;
        cachedStdErr: Array<string>;
        _xterm_cdn_base_url = 'https://cdn.jsdelivr.net/npm/xterm@5.1.0';

        constructor() {
            super();
            this.cachedStdOut = [];
            this.cachedStdErr = [];

            // While this is false, store writes to stdout/stderr to a buffer
            // when the xterm.js is actually ready, we will "replay" those writes
            // and set this to true
            this._moduleResolved = false;

            //Required to make xterm appear properly
            this.style.width = '100%';
            this.style.height = '100%';
        }

        async connectedCallback() {
            //guard against initializing a tag twice
            if (knownPyTerminalTags.has(this)) return;
            knownPyTerminalTags.add(this);

            this.outElem = document.createElement('div');
            //this.outElem.className = 'py-terminal';
            this.appendChild(this.outElem);

            this.setupPosition(app);

            this.xtermReady = this._setupXterm();
            await this.xtermReady;
        }

        /**
         * Fetch the xtermjs library from CDN an initialize it.
         * @private
         * @returns the associated xterm.js Terminal
         */
        async _setupXterm() {
            if (this.xterm == undefined) {
                //need to initialize the Terminal for this element