-
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
Typescript build #247
Typescript build #247
Conversation
8bb0278
to
bc2a596
Compare
I think that going with TypeScript is the best choice right now. I have some comments/questions though, before we get a decision on this:
Would changing the addons directory first help out with this (#250)? Demo crash screenshot |
|
I am on SourceLair and the build command succeeded
Hopefully tomorrow we will settle down on this and we will be ready to move on. Would I help if I moved the |
@parisk looks like it's failing for the same reason as the test is failing; weirdnesses with |
@parisk it might be wise to add |
Ok the tests should be passing now, I moved the addons over but kept them in js for now so the build step is just copying them over. My main concern now is that I'm not sure the best way to have git ignore all changes within the |
Hmm, after diving a little into it, i changed my mind, i'll stick with the es6 version. As i can see it now, the typescript version is kinda pointless (as the main Terminal.js is still in pure JS, and the overall typescript brings, for now, unnecessary complexity - we do not gain anything, for now) I'll continue to work in the ES6 version and rewrite Terminal.js as ES6 module. |
Of course, that means a deep refactoring of Terminal.js internals and brings us totally out of scope for the #158 |
Also, for the global "document" & "window" scope, this is accidental complexity and is "fixed" in my PR Basicaly, Terminal should not depends on document scope at ALL, until been attached with terminal.open(domElement) , follow this and you 'll simplify both the tests & code. |
@Tyriar maybe we should separate the directories for temporarily built files and distributed builds. What if we kept temporary built files in |
Please wait for a minute. I think I am preparing a quite simpler PR. |
It seems that when I was referring to a minute, I meant 3 hours... I started getting scary with the magnitude of these PRs, so I started a new smaller one (strictly less than 200 additions), in order to move things forward gradually, instead of all at a time. Could you please take a look at #251? |
Fixes #158
@parisk this is the first run at the introduction of a TypeScript build system. Here is the overview of changes:
The Terminal object is now exposed as
Xterm.Terminal
.Running
npm run build
will run the code through TypeScript to generate commonjs modules and then through browserify to squash the modules, watching can be added in the future via watchify.CompositionHelper
went through a full typing conversion, so use this as an example of what "real TypeScript" code looks likeTerminal
interface at the bottom allowing strong typing without needing a reference to theTerminal
object, eventually this would be moved to another file and implemented byTerminal
in terminal.ts.this
to continue to be used.Viewport
was refactored into a class without types, so use this as an example of a bare-bones conversion.any
is used to indicate that the type could be anything which relaxed the TypeScript compiler around usage of that variable/param/etc.Terminal
was retained in a js file and pulled in viarequire
, I made an attempt to do this conversion but the file is just so damn big I wouldn't be confident in the result even if it did work.Terminal
would have to be slowly refactoring into several modules. Check out this branch for a preview of what that looked like. An example of why this is such a big effort is how messy the code is, the following for example weren't initialized at the top like most ofTerminal
's members:TypeScript will allow us to clean this up in an effective way as errors go off for unused/uninitialized variables.
Because
Terminal
is in a js file, the build command is a little messy at the moment, this would be simplified when it's fully converted.xterm.js
is the entry point which provides the TypeScript classes to the (JavaScript)Terminal
typings/require.d.ts
is a "typings file" which is a TypeScript concept that provided type safety to external libs, in this case therequire
function. This can also be included via thetypings
npm module.tsconfig.json
specifies the compile configuration, for example you can specify the type of module (amd, umd, commonjs, es6, etc.) as well as input and output files/dirs.These things would need to be followed up after this change:
Terminal.loadAddon
is still needed, if so make sure it works