Simple TypeScript Support

[ikarienator]

Concerning Feature request: TypeScript support Supports:

• Simple syntax highlight.
• Type message when hovering
• Member completion
• Navigate to definition

[ikarienator]

I also set up a demo at: http://twbs.in:7261/editor/src/render/Program.ts#0,0

[aclement]

This is awesome, thanks! Demo site seems to have gone down for me unfortunately. I'll have a play with it tomorrow.

[ikarienator]

@aclement Yeah I put too many contents on the VPS. I restarted it just now.

[ikarienator]

@aclement I check the log and found this you may be interested:

(node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit.
Trace
    at Socket.EventEmitter.addListener (events.js:160:15)
    at Socket.Readable.on (_stream_readable.js:653:33)
    at Socket.EventEmitter.once (events.js:179:8)
    at TCP.onread (net.js:527:26)
(node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit.
Trace
    at Socket.EventEmitter.addListener (events.js:160:15)
    at Socket.Readable.on (_stream_readable.js:653:33)
    at Socket.EventEmitter.once (events.js:179:8)
    at TCP.onread (net.js:527:26)
(node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit.
Trace
    at Socket.EventEmitter.addListener (events.js:160:15)
    at Socket.Readable.on (_stream_readable.js:653:33)
    at Socket.EventEmitter.once (events.js:179:8)
    at TCP.onread (net.js:527:26)
FATAL ERROR: Evacuation Allocation failed - process out of memory

[aclement]

Sorry I didn't get to this sooner, I was at EclipseCon last week and tied up with so much stuff. I'll be taking a look this week

[aeisenberg]

I'll take a look at this.

[aeisenberg]

This is some great work. Thanks for contributing some great functionality that really should be in scripted. Before we can proceed with merging, I would like some clarifications and changes:

Licensing and IP questions:

1. Please include license headers on all files that do not have them yet. They should be made available under an EPL license using a header like this (but use your own name and affiliation):

   /*******************************************************************************
   * @license
   * Copyright (c) 2013 VMware, Inc. All Rights Reserved.
   * THIS FILE IS PROVIDED UNDER THE TERMS OF THE ECLIPSE PUBLIC LICENSE
   * ("AGREEMENT"). ANY USE, REPRODUCTION OR DISTRIBUTION OF THIS FILE
   * CONSTITUTES RECIPIENTS ACCEPTANCE OF THE AGREEMENT.
   * You can obtain a current copy of the Eclipse Public License from
   * http://www.opensource.org/licenses/eclipse-1.0.php
   *
   * Contributors:
   *     Andrew Eisenberg
   ******************************************************************************/

2. The remaining files, I'm guessing, are Microsoft files and are released under ASL. We have some friendly lawyers who can figure out how we can use these files. It may be that they can't reside directly in the repository, but must be installed separately using npm.

Functionality questions:

1. Hovers and navigation are working great for me (yay!). However, content assist appears to be returning no results. I haven't looked into why yet. Will follow up.
2. The editor feels a bit sluggish. I wonder if this is because there is a lot of work going on as you type. Some of this should be pushed to a webworker to free up the UI event loop.
3. Navigation to a definition of a variable highlights the entire statement not just the name. Eg- perform navigation on the second user and the entire first line is highlighted: var user = "Jane User"; user
4. I see that you have jshint (or would they be tshint???) annotations being displayed as you type. That's great and it's something I would like to see for scripted JS files, but I am concerned that this is contributing to the feeling of sluggishness. I would probably change this to only updating on save and seeing if this helps somewhat.

Implementation questions:

1. It looks like you are hooking into the typescript compiler to get the references, hovers, definitions, etc. I am cautious about adding a dependency on a third-party library, but I thin kin this case it makes sense. We will probably not be able to store this in our repo (see above) and will need to move it to an npm dependency.
2. You are using built-in typescript dependency loader. Once a file is loaded, does the dependency stay in memory? Are dependencies grabbed from the server on each inference invocation? Neither of these solutions are likely scalable. We will probably need to store some sort of module summary to local storage as we currently do for JS files. This way files do not need to be re-requested and re-parsed on each inference invocation, nor will memory be overwhelmed by files that take lots of memory. We will have to experiment a bit to make sure, though. I'm guessing, though, that this is also a contributing factor to the sluggishness.
3. There is obviously a build step to translate from ts to js. What is that step and can a compile script be included in the repo? We can probably add an exec command in the .scripted file to automatically run the build whenever a .ts file changes.

Not all of these problems need to be fixed before we can merge. At a minimum, we need to have the ip issues resolved, as well as ensure the build step is seamless. Thanks again for your work.

[ikarienator]

Many thanks for the review!

1. I didn't even know there would be an IP problem until today. I though Apache means "use freely". I think I'm quite wrong about that.
2. I think the sluggishness comes from the recompile. I'm incrementally recompiling every 1 second in the main process. :( I can move it to the worker process, but may need some rework though.
3. TypeScript compiler is a little bit huge and not easy to hack on. I didn't find a way to create summary instead of including all the files in the service host. Maybe it's possible or maybe we have to create our own compile unit management functionality and use only the parser provided by Microsoft. It is also possible to make the indexer running on the server side and exchange only the summary to the browser. This might also work on JavaScript editing too.

[aeisenberg]

We don't need to work on all of the performance issues before this can be merged, but I do fear that the plugin would be unusable for any real files and projects without a more asynchronous and background compilation step.

You do need to add proper license headers for each file. Licensing is quite a tricky thing to do properly. EPL does allow you to use and create derivative works, but the license header must remain in tact and the source code must always be available. Furthermore, there needs to be some due diligence done to ensure that the code was not borrowed from a proprietary source (note- I am NOT accusing you of doing anything nefarious, these are just standard procedures for many open source projects).

Also, mixing code with different licenses is problematic since they have different restrictions on what you are allowed to do. So, we will not commit the MIT or ASL-licensed code into the repository. This is standard practice for most open source projects.

What this means for you is this, before I can integrate your patch:

1. Create a new commit that removes all code that is MIT licensed
2. Add EPL license headers for all files that you wrote yourself
3. Document the build step in a readme file that you will also commit
4. Let me know where to find the version of the typescript sources that you are using.

Once this is available, I can merge this request.

[ikarienator]

Thanks for the explanation! I will be working on that soon. BTW, do I need to add headers to generated files as well? If we do, I will edit the build script to add it.

The included typescriptServices.js and typescriptServices.d.ts are compiled from the source code found here: https://typescript.codeplex.com/releases/view/98308 It can also be found in the npm package typescript 0.8.3.

[aeisenberg]

All files to be checked into the repo require a license header. So, yes, the generated files need them as well since we will likely check them into the repo. And please include the build script.

I'll take a look at the npm scripts. We actually use bower for our client side dependency management, so I'll see if it is already in the bower repo. If not, I can add it.

On Thu, Apr 4, 2013 at 9:21 PM, Bei Zhang - Ikari Enator < notifications@github.com> wrote:

Thanks for the explanation! I will be working on that soon. BTW, do I need to add headers to generated files as well? If we do, I will edit the build script to add it.

The included typescriptServices.js and typescriptServices.d.ts are compiled from the source code found here: https://typescript.codeplex.com/releases/view/98308 It can also be found in the npm package typescript 0.8.3.

— Reply to this email directly or view it on GitHubhttps://github.com/scripted-editor/scripted/pull/266#issuecomment-15938347 .

[ikarienator]

I'll close this PR for now. Will create another PR soon as I finished it.

[bestander]

@ikarienator it would be great to see your TypeScript support in scripted. I hope you'll have time to finish it :+1: