Skip to content

problem with SerializeAddon and globalObject #4424

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

Closed
PerBothner opened this issue Feb 28, 2023 · 5 comments
Closed

problem with SerializeAddon and globalObject #4424

PerBothner opened this issue Feb 28, 2023 · 5 comments

Comments

@PerBothner
Copy link
Contributor

I don't think this is a bug, but there is an inconsistency between the serialize (and unicode11) addons vs the other addons, because of PR #3421. The webpack.config.js files for serialize and unicode11 contains

globalObject: 'this'

but the other addons don't.

To load them I do:

const XTerm = window.Terminal;
const CanvasAddon = window.CanvasAddon.CanvasAddon;
const FitAddon = window.FitAddon.FitAddon;
const SerializeAddon = window.SerializeAddon.SerializeAddon;
const WebglAddon = window.WebglAddon.WebglAddon;

This works if I remove the globalObject: 'this' from xterm-addon-serialize/webpack.config.js. If I don't remove that line, the SerializeAddon fails to load. I don't know what to replace the window.SerializeAddon.SerializeAddon with - replacing window by this doesn't work.

I'm not sure how to fix this. I'm not very knowledgeable about webpack or umd modules.

I can run webpack "manually" (I'm actually using autotools and make to wrap everything) to override the globalObject that way. Or perhaps generate EcmaScript modules (which I would prefer anyway), so one idea to run webpack with overrides to generate Es6 modules (type: module`).

BTW: Using the Serialize plugin enables both tmux style session attach/detach, and drag-and-drop of a termnal from one top-level window to another. Both pretty valuable features.

@jerch
Copy link
Member

jerch commented Feb 28, 2023

Does it work, if you just remove the window? Like this:

// ctors
const XTerm = Terminal;
const CanvasAddon = CanvasAddon.CanvasAddon;
const FitAddon = FitAddon.FitAddon;
const SerializeAddon = SerializeAddon.SerializeAddon;
const WebglAddon = WebglAddon.WebglAddon;

Background on this madness - JS has a hard to understand mutation rule for the this reference:

> function bb() {return this};
< undefined
> bb()
< Window {window: Window, self: Window, document: document, name: '', location: Location, }
> a={};
< {}
> a.bb = function() {return this;}
< ƒ () {return this;}
> a.bb()
< {bb: ƒ}

In the console.log example above function bb is defined on top level and also called from there - this in execution context of bb points to window.
Something magically happens for a.bb though - this got replaced with a reference to the object a.

Javascript basically calls any function by populating the first argument depending on deriving context, a C equivalent would roughly look like this (since you know C):

This* bb(This* this) {
  return this;
}

// calling bb() directly
bb(window);
// calling a.bb()
bb(a);

The reason why Javascript shapeshifts this comes from its OOP implementation - other than in class/blueprint based OOP, Javascript is prototype based, eg. you start from some prototype object and extend it with your functionality further down in the prototype chain. The this shifting is Javascripts way to solve the data to logic coupling to allow some sort of encapsulation (its really loosely coupled only, as you can easily override this from a.bb.call(some_other_obj)).
There is more to it (like arrow function dont mutate this anymore etc), but thats the basic gist of it.

Now what does it all have to do with the global object?
First - the global object is the grandfather of all "thisses", if there is none to derive from deeper in the chain, the global object gets applied.

In the early days of good old vanilla ECMA3 everything from foreign scripts got hooked into the global object and was accessible from there. The loading is quite simple - the foreign script would see the same top level context, thus anything defined there on top level would get hooked to that one and only window object. This leads to issues for bigger applications loading tons of 3rd party scripts, maybe with clashing/overwriting previous definitions. So ppl invented module encapsulation, and the idea of an exports object with its name as namespace was born (nodejs' way to symbolize imported stuff).

Next engines started to implement different global objects and names - global on nodejs, self in web worker, beside window in classical mainthread browser context. Urgh - different global objects - how to access them from deep within?

Additionally several module loaders were invented (not speaking of ESM, it works quite differently), often putting their loading mechs into functions or methods of its loader objects. But deep nested logic means - its coupled to a custom this object, again tricky to access the real global object for loader, that wants to put things into global scope.

(There is some relief - an additional global reference globalThis guaranteed to point to the real top level object from any nesting. Not sure if all engines already implement that...)

Back to you issue with this
The this applied from module webpack build is not the same as your this where you try to access it. Idk what webpack tries to apply here, maybe look into the module and check, what the exported object gets bound to? If that indeed this.AddonXY = {...} then it depends how you load it. If done on toplevel (eg. from a script element), then it should still reside on window.

Sorry for the wall of text.

@PerBothner
Copy link
Contributor Author

PerBothner commented Mar 1, 2023

Does it work, if you just remove the window? Like this:
const SerializeAddon = SerializeAddon.SerializeAddon;

Doesn't seem to help.

As an experiment, I modified xterm-addon-serialize/webpack.config.js to build es6 modules:

diff --git a/addons/xterm-addon-serialize/webpack.config.js b/addons/xterm-addon-serialize/webpack.config.js
index 7c0ccd01..95cb07b1 100644
--- a/addons/xterm-addon-serialize/webpack.config.js
+++ b/addons/xterm-addon-serialize/webpack.config.js
@@ -24,9 +24,10 @@ module.exports = {
   output: {
     filename: mainFile,
     path: path.resolve('./lib'),
-    library: addonName,
-    libraryTarget: 'umd',
-    globalObject: 'this'
+    library: { type: 'module' }
+  },
+  experiments: {
+      outputModule: true,
   },
   mode: 'production'
 };

That allowed me to do what I really want to do:

import { SerializeAddon } from "./xterm-addon-serialize.js";

One idea would be to add to each addon a webpack.for-esm.js to build EcmaScript modules, with the output files in (say) lib-esm. Also, a package.json rule to run that. While my node/webpack skills are limited, I can probably figure out how to do that - if it is likely to be accepted. Otherwise, I can write add to my Makefile some sed magic to create a webpack.for-esm.js for each addon on-the-fly.

@jerch
Copy link
Member

jerch commented Mar 1, 2023

To make sure I understand the issue - does it work for you with a simple <script src="path/to/serialize/addon.js"></script> in document head, and accessing it later as new SerializeAddon.SerializeAddon() in JS? Because that should def. work - if it doesnt, then plz show us how you are loading the addon (or the bundle is seriously broken).

ESM is currently not supported w'o re-bundling things yourself, yeah. We have that on the TODO list for quite some time, but it is abit involved for xterm.js, due to complicated TS setup and different browser|headless build targets.

The addons though might be a good starter, as they are quite simple in the module structure. In #2878 I suggested something similar to your double bundling idea - ideally we manage to keep the old package format intact to not break current integrations, but just extend the packages with ESM libs and proper export annotations in package.json.

@PerBothner
Copy link
Contributor Author

This is weird. Presumably an artifact of module loading.

The following doesn't work:

const FitAddon = window.FitAddon.FitAddon;
const SerializeAddon = SerializeAddon.SerializeAddon;
class XTermPane extends DTerminal {
    constructor(windowNumber) {
        super(windowNumber, "xterminal");
        this.fitAddon = new FitAddon(false);
        this.serializeAddon = new SerializeAddon();
        this.rendererType = 'canvas'; // 'dom' 'canvas' or 'webgl'
    }

but this works:

const FitAddon = window.FitAddon.FitAddon;
//const SerializeAddon = SerializeAddon.SerializeAddon;
class XTermPane extends DTerminal {
    constructor(windowNumber) {
        super(windowNumber, "xterminal");
        this.fitAddon = new FitAddon(false);
        this.serializeAddon = new SerializeAddon.SerializeAddon();
        this.rendererType = 'canvas'; // 'dom' 'canvas' or 'webgl'
    }

Except for the const SerializeAddon and changing the 'new to new SerializeAddon.SerializeAddon() these are the same. Note that loading FitAddon works.

The reason I used the top-level const declarations to keep the "import-related" code in one place. However, it's not that important.

The new SerializeAddon.SerializeAddon() approach is good enough for now. We can close this issue, if you agree.

@jerch
Copy link
Member

jerch commented Mar 1, 2023

Eww, thats indeed very weird. On which engine did you find that, is that qt embedded QJSEngine? And are you sure, that at the time of your const binding the module is readily loaded (type="module" in script tags)?

Reasoning behind my questions:
On a first sight your issue looks like the SerializeAddon.SerializeAddon cannot be rebound. But JS has not really a concept of this, unless the engine makes some really weird symbol setup stuff, which may depend on how you get the modules into the JS context. The QJSEngine has an injection method for symbols coming from C++, if I remember right those cannot be bound to other names, and you'd always have to point to your one and only injection name. (Used that years ago to get a console.log impl, at that time the engine had no builtin for console yet.)

Still there is another possible explanation - if you use ESM semantics at the script element (type="module"), the symbols will be only visible within that script context, but not in the global context anymore. Here setting globalObject: 'window' in webpack config might work around that and still expose the addon content correctly. But it also hides the fact, that ESM loading is concurrent, thus you might run into the same issue from other addons erratically (basically a race condition between module loading + symbol binding vs. you grabbing the symbol in top-level context).

Gonna close the issue - still, if you want to dig deeper, I'd really be interested in the real issue behind that, as it raises more than one eyebrow.

@jerch jerch closed this as completed Mar 1, 2023
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

3 participants