Skip to content
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

Move to typescript #85

Closed
Mpdreamz opened this issue Apr 2, 2015 · 14 comments
Closed

Move to typescript #85

Mpdreamz opened this issue Apr 2, 2015 · 14 comments

Comments

@Mpdreamz
Copy link
Member

Mpdreamz commented Apr 2, 2015

Atom will/already does support .ts files OOTB now!

atom/atom#5898

we could try https://www.npmjs.com/package/coffee-script-to-typescript to do the conversion automatically.

I personally like coffeescript/typescript equally but the overwhelming consensus in #omnisharp on jabbr is in favor of TS so lets move this sucker over 😄

@markrendle
Copy link

👍

I suspect that transpiler might be more trouble than it's worth, and it would be easier to compile to JS and then TypeScriptify it. But it couldn't hurt to try it first.

https://github.com/borisyankov/DefinitelyTyped/blob/master/atom/atom.d.ts

@willl
Copy link
Member

willl commented Apr 2, 2015

So what's the action plan? Create a new branch for the TypeScript version? What needs to be done, and what can the rest of us do to help?

Based on observation of the earlier #omnisharp conversations, the move to TS would potentially mean more contributors to this package. It seems unfamiliarity towards coffeescript was/is a factor for some.

@basarat
Copy link
Contributor

basarat commented Apr 2, 2015

@willl asked me to voice my opinion so here it is:

Considering the number of files (under 50) I would be tempted to just rewrite files by hand on by one. I've been doing that when porting over other peoples coffeescript stuff to typescript. Consider:

https://github.com/OmniSharp/omnisharp-atom/blob/master/lib/omnisharp-atom/features/rename.coffee

I would just rename to rename.ts and start hacking:

var _ = require('underscore')
var RenameView = require ('../views/rename-view')
var Omni = require ('../../omni-sharp-server/omni')

class Rename {
   // You get the idea. 
}

export = RenameView

For views I would leave the ones that are okay in Coffee e.g. : https://github.com/OmniSharp/omnisharp-atom/blob/master/lib/omnisharp-atom/views/omni-output-pane-view.coffee

You can even leave the html portion in a coffeescript file that your ts view requires : e.g. https://github.com/TypeStrong/atom-typescript/blob/master/lib/main/atom/views/renameView.ts#L3
and coffee https://github.com/TypeStrong/atom-typescript/blob/master/views/renameView.html.coffee
Note: I was still messing around at that time. And I would (and did) place the html next to the ts file but collabs wanted it the way it is, i.e. all coffee in one place : https://github.com/TypeStrong/atom-typescript/tree/master/views

The thing that will absolutely drive you up the wall is heavy use of ? in coffeescript. The TS alternative is extremely verbose to write out ( vote on this : microsoft/TypeScript#16 )

Later when you have more stuff in .ts you can start changing var/require to import from

🌹

@ctolkien
Copy link
Member

ctolkien commented Apr 3, 2015

I'm completely green with typescript as well, but I'd support this migration! 👍

@david-driscoll
Copy link
Member

I'd be willing to help as well.

@nosami
Copy link
Contributor

nosami commented Apr 6, 2015

@willl, @david-driscoll and @ctolkien have offered to migrate this over. Why don't we make them contributors so they can collaborate more easily?

@will
Copy link

will commented Apr 6, 2015

Please do not make me a contributor

@nosami
Copy link
Contributor

nosami commented Apr 6, 2015

@will - oops, fixed :)

@will
Copy link

will commented Apr 6, 2015

:)

@nosami
Copy link
Contributor

nosami commented Apr 6, 2015

I suggested on JabbR that we make a list similar to this one (OmniSharp/omnisharp-roslyn#29) with each file listed. Then when someone wants to work on that file, they can just put their name next to it.

@basarat
Copy link
Contributor

basarat commented Apr 6, 2015

@willl
Copy link
Member

willl commented Apr 9, 2015

It's happening #98. :)

@willl
Copy link
Member

willl commented Apr 23, 2015

Current code in master is now in TypeScript now that 0.3.0 has been released. Contributions always welcome :)

@david-driscoll
Copy link
Member

Since we pushed 0.4.0 as typescript, I think we can close this! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants