-
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 some build system to enable more than one js file #158
Comments
I totally agree with that. Let' get on top of this after finishing documentation and releasing 1.0.0. |
I suggest webpack (prototype here) as I've used it before and it does exactly what we want (bundles several files into 1 using |
Another option is TypeScript which I've become quite fond of after working on vscode. This would allow multiple files to be built into one and provide type safety which would promote code quality (especially good since unit tests are patchy). |
I think that plain ES6 or TypeScript are more suitable for this. Webpack seems to be more of an overkill at this moment. The most important thing that we have to take into account is how are going to ship the code here. This most probably will introduce breaking changes (especially for Bower users), since we will need to find a way to ship the built artifacts to keep this library working in the browser out of the box. I am adding this to the "open" milestone for version 2.0 and start some experimentation on this, preferably with plain ES6, which is close to TypeScript. My main concerns with TypeScript are:
|
When using bower most projects with a build system add the built files to a dist directory and those are the ones you link to. It would introduce a breakage though so v2 would be appropriate. |
+1 for using Typescript. No clue about the current state of ES6 support, I think generators are still at an early stage in some JS engines and therefore not usable yet. |
I'll assign myself to put together a TypeScript prototype. |
May i suggest you start with ES6 & Babel ? Leveraging typescript is a good thing, yet, it will come with a price. Having an es6 project will allow your user to just "var Terminal = require('xterm')" and use the raw source code (browserify will work just fine)". When with typescript, you'll have a different file for the build and the source of your application. Also, thanks to typescript design, an ES6 project is, de factor, compatible typescript. The full migration to typescript can come in a second (and very smooth) step. May i help here ? |
Sourcemaps gets around that if needed though, the type safety is the main benefit imo. |
This is not about the sourcemap, but having no "build" step required for thoses who just use browserify (ES6 is already supported in most modern browser) |
@131 due to bower we would likely be committing the compiled version to a |
Is it ok for me to give a try to a es6 version ? (i played a little) |
The most promising changes in ES6 cannot be polyfilled by babel - the iterator/generator/comprehension concepts. Downvote for the vanilla ES6 idea, because of the current state of ES6 in commonly used JS engines. And still hoping for some optional type annotations in some later JS versions... |
Babel support for generator is complete (babel preset es2015) and works perfectly with with co, yet, current code do not gain much of those. I've a branch ongoing #245 As i'v ditched fetch, no promise are left for now (so it's hard to do a demo of generators), but if you want, i can easily set up a working version with generator & promise (just tell me on what event / operation ) |
Okay, so let me contribute to this. First of all let's agree that in version 2.0 (after we migrate to either option):
These are prerequisites to make usage of this library as easy as possible, while making it more maintainable. Next, let me answer to as many comments as I can: About Generators
@jerch as @131 mentioned, generators are fully supported in the babel polyfill: http://babeljs.io/docs/learn-es2015/#generators. This does not concern us in any case though, since we are not using generators right now in any way and there is no open issue that could benefit from generators. So either way we won't be using them any time soon (until we have to). About importing (TypeScript vs ES6)
@131, I agree with @Tyriar here. Anyone who will consume xterm.js will have access to the built files since they will fetch a release of a library published in npm, bower or GitHub with the built artifacts included. About TypeScript vs ES6While personally I am not settled yet on which is the best solution, I lean slightly towards vanilla ES6 for the following reasons (most of them mentioned in #158 (comment)):
Despite the above arguments though, I am really open to moving to TypeScript, since building a code base that is reliable and future proof is always a good idea. We just have to weight the pros and cons. Last, as I have no significant experience with ES6 and TypeScript, I would ❤️ to see both prototypes and contribute to the conversation of both. So thanks a lot both @Tyriar and @131 for offering to create prototypes 🎉 . P.S.: I started reading about TypeScript and ES6 already, in order to provide some more constructive comments and help take a proper decision. |
I'v updated my PR, with a working /dist/ folder (with a working client side demo in it, no server required). , please look at the repository tree (the PR is not so nice to read). Addons are also available. npm run start for server side demo is also working. Regarding @parisk, for 1: the public API for importing this library will break @Tyriar , @parisk , @jerch i guess i'm done, now, i'll happily have your feedback ! |
On addons, I think they should be split out to other repos in the xtermjs org, this makes including them easier (install the dep and require it), promotes separation and makes them more clear examples on how to write an addon. |
Having said that, they currently use private APIs so if this was the case we should expose public APIs that enable them to do this sort of stuff. |
TypeScript PR is up at #247 |
Ah well, my info about the generator support of babel seems to be outdated. Why I came up with it - generators as coroutines are a nice way to work with streamline data. This technique is often used for code separation in Python. (I think the generators in ES6 are heavy influenced by Python.) xterm.js could benefit from this by splitting the Terminal object into pieces of responsibilities. Especially the parser could be separated from terminal state handling easily with a generator pattern. It would reduce the fat switch statements in To illustrate this further, the way
What makes this hard to maintain is the deep nesting of the terminal state handling within the parser states in one big method. Also it is not guaranteed that on a deeper level the parser state gets not modified (it actually happens). With generators this can be rewritten to:
This can be even more simplified if the parser aggregates the ANSI codes to basic actions a terminal has to support (internal API) and leads to a complete separation of ANSI code parsing and terminal handling:
With a well defined set of actions the parser and the terminal can be developed and tested independently. Since a full escape code parser model with actions got already specified elsewhere (http://vt100.net/emu/dec_ansi_parser) it is quite easy to adopt it. |
Hi @jerch, thanks to your advices, i'm willing to give a try, i'll get back to you soon |
@131 Oh ok, this will be a major rewrite of |
Interesting discussion happens also at codemirror/codemirror5#4200. |
Yep, i dont use defaut export too. |
We use rollup for building, and plan to use buble for ES6 transpiling. |
Hi @adrianheine, I saw that your PR in CodeMirror 👍 . Great job. Rollup worked amazingly well, except for the testing part. I couldn't find a way to make it work with Mocha and I didn't want to use different transpilers (rollup and babel) for output and tests. Any ideas? |
Introduce Babel as the build system, convert src/xterm.js to ES2015 and export EventEmitter
It's difficult to simplify the code base if it all sits in a 5000 line file.
The text was updated successfully, but these errors were encountered: