-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Introduce Babel as the build system, convert src/xterm.js to ES2015 and export EventEmitter #251
Conversation
|
In the last commit of my PR, i'v preserved the /src/ folder as it was in the master branch, so now #245 provides :
|
|
||
# Watch for file changes inside the `src` and `addons` directories. | ||
# Every time a file changes, then build xterm.js and restart the demo server. | ||
node_modules/.bin/nodemon --watch src --watch addons --exec bash -c "./bin/build && node demo/app" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any non-trivial scripts like this should be put directly in scripts for cross-platform compatibility.
7b146d6
to
edc3a25
Compare
edc3a25
to
ed1a31d
Compare
🆗 folks, this should be considered ready. Feel free to take a look, in order to merge this and move forward with further fixes. |
Ship it 👍, I'll make sure it works in vscode in a few days to a week. We're going through our testing/release phase now so this will probably hit the Insiders build in a week or so. |
Awesome. I will move on with creating some issues to further improve the codebase after this. |
This PR introduces the following major changes in the codebase:
src/xterm.js
to an ES2015 native moduleand eventually just closes #158.
Goals
Details
bin/build
is introduced to build xterm.jsbuild/
directory, which is ignored by Gitdist/
directory and are committed into the repositorybin/demo-server
is introduced to server the demoKnown issues
The constructor class of xterm.js is now accesses throughThis changed and now the Public API ofxterm.Terminal
. This is valid across all module systems (CommonJS, RequireJS and plain browser environment).Terminal
stays the same.dist
directory instead ofsrc
What I would like to achieve, but I did not manage to do is maintain the same public API for xterm, but there was no way I could get around this with browserify and UMD export.
Rollup worked great with this (before switching to babel), but it completely messed up the tests and I couldn't fix them.
This PR is greatly inspired by the great work done in #245 and #247 by @131 and @Tyriar.
While both of the above PRs are great, both of them included additional code and concepts that were not described in #158 (e.g. introducing TypeScript for type safety, or changing the communication protocol of the demo) and introduce complexity in reviewing of both PRs.
As a result I would like us to address all those topics in additional standalone issues, in order to be certain about our choices.