Xterm.js: Generate esm output for xterm.js

[torch2424]

Hello!

Currently, we are using Rollup to handle our bundling. We recently upgraded from 3.14.5 to 4.10.0, and got the error of:

I'll admit I didn't verify if it is correctly giving the name export since node_modules/xterm/lib/xterm.js is minified, or this honestly may be an issue with Rollup. But if I'm not mistaken, generating an esm build would also allow for better tree shaking, which according to bundlephobia currently is not supported.

Thanks! I'm open to opening this PR myself if needed, as I think it's a small change in the webpack config 😄

Looking forward to the response!

[Tyriar]

We build it as a UMD module:

https://github.com/xtermjs/xterm.js/blob/631107698ec1f28577101cc570540f097a83af68/webpack.config.js#L38

It looks like rollup supports it, we simplified the build a lot with v4 and combined the old direct TS output (lib) and the browserified output (dist) into a single umd module.

[torch2424]

@Tyriar Thanks for the response! 😄

Oh yeah so I noticed it is currently output as a UMD. The fact that it doesn't work is probably a rollup issue. I'll dig a bit deeper once I get the chance 😄

That being said, I still think the esm would be worthwhile the extra build step for tree shaking. As, in theory, it could greatly reduce bundle size for a lot of apps. 🤔

Also, it would allow use to support the module field in package.json, instead of only main.

No worries if you don't see the value in it, was a suggestion for the project, and totally open to discussing it more. Let me know if there is something specifically I can do to provide more info, or help.

Thank you! 😄 👍

[Tyriar]

It doesn't look like module is supported/documented by npm? https://docs.npmjs.com/files/package.json

[mofux]

I've run some quick tests to see if we can support a module build relatively easy.

Unfortunately typescript itself does not support translating our absolute imports into relative ones (and there is not much hope that they will do soon: https://github.com/microsoft/TypeScript/issues/33118#issuecomment-542030685). There are hacks to get this working, but I'd rather not rely on them as they all seem very fragile.

The other option would be to use webpack to create the esm build for us, but unfortunately webpack does not support modules as a library target yet. There is hope that the not yet release webpack v5 can bring support for it: https://github.com/webpack/webpack/issues/2933

I'd say let's revise this topic in a few month time and see if support in either webpack or typescript has been added.

[torch2424]

@Tyriar

It doesn't look like module is supported/documented by npm? https://docs.npmjs.com/files/package.json

Ah yes, I don't think npm highlights it, since they focus on the "node" use case? But it is a common pattern. For instance, looking at comlink, their npm package has the module field and bundlers will respect it

@mofux

I've run some quick tests to see if we can support a module build relatively easy.

Ah, thank you very very much for looking into this! 😄

Unfortunately typescript itself does not support translating our absolute imports into relative ones (and there is not much hope that they will do soon: microsoft/TypeScript#33118 (comment)). There are hacks to get this working, but I'd rather not rely on them as they all seem very fragile.
The other option would be to use webpack to create the esm build for us, but unfortunately webpack does not support modules as a library target yet. There is hope that the not yet release webpack v5 can bring support for it: webpack/webpack#2933

Ah that's quite unfortunate, didn't know that Typescript and Webpack had troubles with this. Definitely not a smalled fix like I had originally though. Thanks for all the context and looking into this 👍

I'd say let's revise this topic in a few month time and see if support in either webpack or typescript has been added.

Totally, sounds like a good plan to me 😄

@Tyriar @mofux

You think it'd be a good idea to close this for now? Or keep it open for the future? I'm open to either 😄

[Tyriar]

Let's close for now since we're blocked on it, but feel free to open a new issue or comment on this one when https://github.com/webpack/webpack/issues/2933 gets resolved. I think even if TS supported it as a target that wouldn't be enough since we use custom path mappings.

[fatcerberus]

Regarding the import path rewriting, don’t hold out waiting for it to ever happen: https://github.com/microsoft/TypeScript/issues/31643#issuecomment-496995677

:frowning: