Skip to content

Loading via browser and the Universal Module Definition #2941

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.

Sign up for GitHub

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

Jump to bottom
Closed
lachdoug opened this issue May 21, 2020 · 11 comments
Closed

Loading via browser and the Universal Module Definition #2941

lachdoug opened this issue May 21, 2020 · 11 comments
Labels
breaking-change Breaks API and requires a major version bump help wanted type/bug Something is misbehaving

Comments

@lachdoug
Copy link

In Issue #2499 it was asked why addons don't work when loaded by browser from a CDN.

Addons are supposed to work as so:

const t = new window.Terminal();
const a = new FitAddon();
t.loadAddon(a);

But the UMD returns an object, so it's necessary to do this:

const t = new window.Terminal();
const a = new FitAddon.FitAddon();
t.loadAddon(a);

Here's a full example:

<!DOCTYPE html>
<html>
<head>
	<meta charset="utf-8">
	<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/[email protected]/css/xterm.css" />
	<script src="https://cdn.jsdelivr.net/npm/[email protected]/lib/xterm.js"></script> 
	<script src="https://cdn.jsdelivr.net/npm/[email protected]/lib/xterm-addon-fit.js"></script> 
</head>
<body>
<div id="terminal"></div>
<script>
	const 	t = new Terminal(),
	 	f = new FitAddon.FitAddon();
	t.loadAddon(f);
	t.open(document.getElementById('terminal'));
	t.write('Hello from \x1B[1;3;31mxterm.js\x1B[0m $ ')
</script> 
</body>
</html>

The UMD conditions are:

if(typeof exports === 'object' && typeof module === 'object')
  module.exports = factory();
else if(typeof define === 'function' && define.amd)
  define([], factory);
else if(typeof exports === 'object')
  exports["FitAddon"] = factory();
else
  root["FitAddon"] = factory();

Perhaps the last line should be root["FitAddon"] = factory()["FitAddon"];.
@jerch
Copy link
Member

jerch commented May 21, 2020

@lachdoug See my warning in #2499 regarding CDNs. Pls lets not spread a bad habit.

@lachdoug
Copy link
Author

This is not a CDN issue, per se. The issue is whether xterm addons should be loadable without a module system, using a <script type="application/javascript"> tag. There are times when it makes sense to load xterm this way. I teach a web dev class to high school kids. It's basic stuff. We use xterm in a lesson about character encoding. Loading libraries into global namespace (yes, from a CDN) is all the students know at this point. NPM and Webpack come later.

@jerch
Copy link
Member

jerch commented May 22, 2020

@lachdoug I know that this issue is not about CDN in the first place, still your snippet contains/shows that, and thats the reason why I called it out. Those snippets are often used as copy blueprints, even by long-standing web developers which should know better. Imho the only usecase, where loading xterm.js exact that way makes any sense, is an in-browser only REPL, that does not try to to talk to an outside shell.

Or from a teachers perspective - combining a slightly enhanced version of that snippet with aspects about web security with proper SRI usage, building a chain of trust etc. would at least give your kids the chance to understand the risk and enable them to decide on their own whether they go the secure or insecure way later on. Giving them this snippet without any security context is like handing a sharp knife to a child without telling about the handle and the blade - it will grab right into the blade.

Ofc its not up to me to tell you how to teach your kids, its just whats on my mind regarding xterm.js and insecure CDN usage.

@lachdoug
Copy link
Author

These kids are 12 and 13. Some of them can't remember how to open their text editors. I'm not going to be talking with them about 'chains of trust'. We're just playing around with a bit of ASCII art.
I'm sorry for mentioning CDNs and teaching. These are irrelevant and have distracted from the trust of the issue, which is: xterm's UMD for addons does not work as documented.
If you don't want to change the UMD, you should at least change the docs. There are lots of beginners pulling their hair out because of this little gotcha.

@Tyriar
Copy link
Member

Tyriar commented Jun 8, 2020

👍 window.FitAddon would be more intuitive.

@Tyriar Tyriar added breaking-change Breaks API and requires a major version bump help wanted type/bug Something is misbehaving labels Jun 8, 2020
@Tyriar Tyriar mentioned this issue Jun 15, 2020
Closed
@fishsaidno
Copy link

+1
This would make things nice & straightforward for simple projects not using a module system.

xterm addons should be loadable without a module system, using a <script type="application/javascript"> tag.

@talkquazi

This comment has been minimized.

Sign in to view
@Tyriar

This comment was marked as off-topic.

Sign in to view
@talkquazi

This comment has been minimized.

Sign in to view
@Tyriar

This comment was marked as off-topic.

Sign in to view
@Tyriar
Copy link
Member

Tyriar commented Dec 15, 2022

Closing in favor of #2878

@Tyriar Tyriar closed this as completed Dec 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
breaking-change Breaks API and requires a major version bump help wanted type/bug Something is misbehaving
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@Tyriar @lachdoug @jerch @talkquazi @fishsaidno