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

Code before:
        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;

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

            this.setupPosition(app);

            this.xtermReadyPromise = this._setupXterm();
            await this.xtermReadyPromise;
            knownPyTerminalTags.add(this);
        }

        /**
         * Fetch the xtermjs library from CDN an initialize it.
         * @private
         * @returns the associated xterm.js Terminal
         */
        async _setupXterm() {
            // eslint-disable-next-line
            // @ts-ignore
            if (globalThis.Terminal == undefined) {
                //xterm module proper
                //eslint-disable-next-line
                //@ts-ignore
                await import(this._xterm_cdn_base_url + '/lib/xterm.js');

                const cssTag = document.createElement('link');
                cssTag.type = 'text/css';
                cssTag.rel = 'stylesheet';
                cssTag.href = this._xterm_cdn_base_url + '/css/xterm.css';
                document.head.appendChild(cssTag);

                //Create xterm, add addons

Review comment (refactor):
nit: I do prefer non-redundant names ... `connectedCallback` as example is a well known ugly name as that's a callback by definition (you literally define a method for that, and the name is not even `connectedMethod`) ... we also use TS in here and I feel like it's obvious any asynchronous callback/method gonna return a promise ... do we really need to duplicate unnecessarily the name *Promise* for anything asynchronous? we don't in any other place and it would be vary ugly if everything *async* ends up with the `Promise` name ... can we call this property `this.xtermReady` instead where it's obvious it's a Promise either by looking at the code (`await` needed) or via documentation plus it's really semantic already and explicit?

Code after:
        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

                // eslint-disable-next-line
                // @ts-ignore
                if (globalThis.Terminal == undefined) {
                    //load xterm module from cdn
                    //eslint-disable-next-line
                    //@ts-ignore
                    await import(this._xterm_cdn_base_url + '/lib/xterm.js');

                    const cssTag = document.createElement('link');
                    cssTag.type = 'text/css';
                    cssTag.rel = 'stylesheet';
                    cssTag.href = this._xterm_cdn_base_url + '/css/xterm.css';