I now need to fork to use an arbitrary commit in vscode

[Tyriar]

After the babel change I now can no longer require xterm.js into vscode as it complains about import.

❯ ./scripts/code.sh                                                                                                                                                           
[5545:0915/162832:ERROR:browser_main_loop.cc(231)] Running without the SUID sandbox! See https://chromium.googlesource.com/chromium/src/+/master/docs/linux_suid_sandbox_devel
opment.md for more information on developing with the sandbox on.                                                                                                             
[5545:0915/162833:INFO:CONSOLE(136)] "Loading "xterm" failed", source: file:///home/daniel/dev/Microsoft/vscode/out/vs/loader.js (136)                                        
[5545:0915/162833:INFO:CONSOLE(137)] "Detail: ", source: file:///home/daniel/dev/Microsoft/vscode/out/vs/loader.js (137)                                                      
[5545:0915/162833:INFO:CONSOLE(139)] "/home/daniel/dev/Microsoft/vscode/node_modules/xterm/src/xterm.js:34                                                                    
import { CompositionHelper } from './CompositionHelper.js';                                                                                                                   
^^^^^^                                                                                                                                                                        
SyntaxError: Unexpected token import                                                                                                                                          
    at Object.exports.runInThisContext (vm.js:76:16)                                                                                                                          
    at Module._compile (module.js:513:28)                                                                                                                                     
    at Object.Module._extensions..js (module.js:550:10)                                                                                                                       
    at Module.load (module.js:458:32)                                                                                                                                         
    at tryModuleLoad (module.js:417:12)                                                                                                                                       
    at Function.Module._load (module.js:409:3)                                                                                                                                
    at Module.require (module.js:468:17)                                                                                                                                      
    at require (internal/module.js:20:19)                                                                                                                                     
    at nodeRequire (file:///home/daniel/dev/Microsoft/vscode/out/vs/loader.js:1908:25)                                                                                        
    at NodeScriptLoader.load (file:///home/daniel/dev/Microsoft/vscode/out/vs/loader.js:1718:37)", source: file:///home/daniel/dev/Microsoft/vscode/out/vs/loader.js (139)    
[5545:0915/162833:INFO:CONSOLE(141)] "Here are the modules that depend on it:", source: file:///home/daniel/dev/Microsoft/vscode/out/vs/loader.js (141)                       
[5545:0915/162833:INFO:CONSOLE(142)] "vs/workbench/parts/terminal/electron-browser/terminalInstance", source: file:///home/daniel/dev/Microsoft/vscode/out/vs/loader.js (142)

Not sure if this is because we're using import incorrectly or maybe it's just not supported by our module loader.

Regardless, after #269 is applied, I then need to manually run npm i && npm run build in order to require xterm.js. So I'm going to need to keep a branch on my fork for the release that I sync to a commit, build and then check in the build files since they no longer ship with the repo and they don't install automatically with the module.

Maybe this doesn't matter in general since most people will not want to ship non-release versions. Wondering if this was considered with the new build and if there is anything we can do to make it easier for anyone to run on an arbitrary commit?

[Tyriar]

Just put this fork together and it seems to work fine https://github.com/Tyriar/xterm.js/tree/vscode-release/1.6

[ebertmi]

I guess you need to transpile the source in order to use it in vscode. VSCode transpiles the TypeScript too, right? Maybe you need to include the xterm source files, so that they get transpiled.

[parisk]

From what I can see in microsoft/TypeScript#1983, TypeScript should be supporting ES6 Modules out of the box. I took a quick look into vscode's codealso and found out you are using the require('module') function though in most cases, so not sure what might be causing this.

Considering a solution. What we could do is:

1. draft weekly releases from master
2. draft a release from every PR merged into master
3. put npm build in the postinstall script, so every time xterm.js gets installed with npm, it builds itself

Not sure though if 1. and 2. will put unwanted weigh into the repository. What do you think @Tyriar?

I am really interested also in learning why this does not work in vscode.

[ebertmi]

I am not keen on the idea of those many releases. If you want to use the repository and not the compiled version from npm, then you should be responsible of compiling yourself.

See #258

[Tyriar]

A release on every PR is a little intense 😅 weekly/fortnightly might be good though, that could be frequent enough (at least when I've been active) to not need to use my fork most of the time. Sometimes it may not be possible though, currently we have an API breakage for example so we need to be sure that everything's all good before committing. It isn't actually that much hassle syncing the fork, just not ideal. It's also a problem pretty specific to our situation since nearly everyone else would be using a tagged release.

I'm also not the biggest fan of compiling in postinstall (#269 (comment)). While it would solve the issue it would also increase install time significantly and there would be problems with Windows as I don't think the build currently works there.

@parisk it's likely more due to the vscode module loader is not expecting it. I don't know too much about the system.

@waywaaard we certainly wouldn't want to include the files in the vscode tree, that would cause a bunch of commit noise and also be more work than the current workaround.

[Tyriar]

Closing this off, we'll finalize the details before we release 2.0.0 but I'm fine syncing the fork for vscode, it's fairly easy to do. Mainly just wanted to bring it to everyone's attention.