Skip to content

Rewrite atom-typescript to use tsserver API #1166

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

Merged
merged 74 commits into from
Mar 20, 2017
Merged

Rewrite atom-typescript to use tsserver API #1166

merged 74 commits into from
Mar 20, 2017

Conversation

guncha
Copy link
Contributor

@guncha guncha commented Jan 12, 2017

What is this?

This is a rewrite of the atom-typescript package to use a different API when talking to Typescript, but with a lot less features compared to the current implementation.

Why would you do such a thing?

Current implementation relies on Typescript's internals, including undocumented APIs, and does a lot of work to match Typescript's behavior. Unfortunately, Typescript's a moving target and this setup is harder to maintain which no one is doing currently.

For example, if Typescript adds a new tsconfig.json option we have to release a new version with updated validation rules. Usually that's trivial, but sometimes it isn't. Supporting something like extends would require a non-trivial amount of work.

More importantly, there are a lot of open issues where the behavior of atom-typescript doesn't match that of running tsc from the command line (closes #1106, closes #1105, closes #1054, closes #1055). There are a few different reasons for those, but they all are caused by atom-typescript trying to replicate what the compiler does and the compiler changing what it does over time.

Fortunately, there's a way to let Typescript handle as many details as possible by using their tsserver API. Unfortunately, this requires rewriting most of the functionality of this plugin.

Pros for this approach

  • Let Typescript itself handle most of the details, like tracking files in a project, finding and reading tsconfig.json files, etc, which makes the plugin more reliable and eases the maintenance burden.
  • Since the tsserver API is mostly backwards compatible and every typescript installation comes with a tsserver executable, we can use the exact Typescript version from the node_modules directory. This remove another cause why atom-typescript and running compiler on the command line might disagree about something. This is also more future proof since you don't need to update atom-typescript with every new version.

Cons

  • Most features need to be re-implemented using this API and some things are not possible altogether since we don't have access to the internals.

Here's a list of things I've implemented so far and others that I haven't:

  • Autocomplete for symbols and types
  • Autocomplete for modules (the default implementation kind of sort of works for imports)
  • Compile on save (defaults to off unless tsconfig.json says otherwise)
  • Error checking (now displayed only with linter)
  • Error checking for tsconfig.json (kind of slow, but works)
  • Build command
  • Build error view
  • “Check all files in project” command (this is new)
  • Rename refactor
  • Go To Declaration
  • Find references
  • References view
  • Format command (though not customizable yet)
  • Quick info
  • Highlight the current symbol in the buffer (GIF?)
  • Symbol view
  • AST view
  • Dependency view
  • “Create tsconfig” command
  • Quickfixes (!!) (these require manipulating the AST that we don’t have direct access to)
  • HTML to TSX command
  • The majority of the typings are now from @types
  • strictNullChecks are enabled for all code

How do I use this?

Simply install atom-typescript-beta package (https://atom.io/packages/atom-typescript-beta). Make sure atom-typescript is disabled as it will not work when both are enabled.

Eventually I'd like to merge this into master, but only if there's enough support from the community so use your 👍 and 👎 on this issue to let me know what you think.

Some GIFs are in order

I've basically smushed all the information into the bottom right of the status bar:
image

Build command:
at-build

Symbol highlighting in the same file:
highlight

Closes #1134

[fix error] Deprecated selector
@UnGast
Copy link

UnGast commented Feb 22, 2017

Thanks to you I can finally use typescript :)

Would it be possible to set intellisense support for files which contain a <script lang="typescript"> tag on the feature list? #1138

@ZuBB
Copy link

ZuBB commented Mar 2, 2017

just got this after installation

/Users/vzuzyak/.atom/packages/atom-typescript-beta/dist/main/atom/components/tsView.js:27
Hide Stack Trace
Error: Failed to execute 'registerElement' on 'Document': Registration failed for type 'ts-view'. A type with that name is already registered.
    at Error (native)
    at Object.<anonymous> (/Users/vzuzyak/.atom/packages/atom-typescript-beta/dist/main/atom/components/tsView.js:27:10)
    at Module._compile (/Applications/Atom.app/Contents/Resources/app.asar/src/native-compile-cache.js:109:30)
    at Object.value [as .js] (/Applications/Atom.app/Contents/Resources/app.asar/src/compile-cache.js:216:21)
    at Module.load (module.js:473:32)
    at tryModuleLoad (module.js:432:12)
    at Function.Module._load (module.js:424:3)
    at Module.require (module.js:483:17)
    at require (/Applications/Atom.app/Contents/Resources/app.asar/src/native-compile-cache.js:50:27)
    at Object.<anonymous> (/Users/vzuzyak/.atom/packages/atom-typescript-beta/dist/main/atom/components/index.js:5:10)
    at Module._compile (/Applications/Atom.app/Contents/Resources/app.asar/src/native-compile-cache.js:109:30)
    at Object.value [as .js] (/Applications/Atom.app/Contents/Resources/app.asar/src/compile-cache.js:216:21)
    at Module.load (module.js:473:32)
    at tryModuleLoad (module.js:432:12)
    at Function.Module._load (module.js:424:3)
    at Module.require (module.js:483:17)
    at require (/Applications/Atom.app/Contents/Resources/app.asar/src/native-compile-cache.js:50:27)
    at Object.<anonymous> (/Users/vzuzyak/.atom/packages/atom-typescript-beta/dist/main/atomts.js:23:1)
    at Module._compile (/Applications/Atom.app/Contents/Resources/app.asar/src/native-compile-cache.js:109:30)
    at Object.value [as .js] (/Applications/Atom.app/Contents/Resources/app.asar/src/compile-cache.js:216:21)
    at Module.load (module.js:473:32)
    at tryModuleLoad (module.js:432:12)
    at Function.Module._load (module.js:424:3)
    at Module.require (module.js:483:17)
    at require (/Applications/Atom.app/Contents/Resources/app.asar/src/native-compile-cache.js:50:27)
    at Package.module.exports.Package.requireMainModule (/Applications/Atom.app/Contents/Resources/app.asar/src/package.js:796:27)
    at Package.module.exports.Package.activateConfig (/Applications/Atom.app/Contents/Resources/app.asar/src/package.js:266:12)
    at PackageManager.module.exports.PackageManager.packageHasSettings (/Applications/Atom.app/Contents/Resources/app.asar/node_modules/settings-view/lib/package-manager.js:51:14)
    at PackageCard.module.exports.PackageCard.hasSettings (/Applications/Atom.app/Contents/Resources/app.asar/node_modules/settings-view/lib/package-card.js:728:34)
    at PackageCard.module.exports.PackageCard.updateSettingsState (/Applications/Atom.app/Contents/Resources/app.asar/node_modules/settings-view/lib/package-card.js:402:16)
    at PackageCard.module.exports.PackageCard.updateInterfaceState (/Applications/Atom.app/Contents/Resources/app.asar/node_modules/settings-view/lib/package-card.js:395:12)
    at /Applications/Atom.app/Contents/Resources/app.asar/node_modules/settings-view/lib/package-card.js:665:24
    at /Applications/Atom.app/Contents/Resources/app.asar/node_modules/settings-view/lib/package-card.js:741:20
    at Function.module.exports.Emitter.simpleDispatch (/Applications/Atom.app/Contents/Resources/app.asar/node_modules/event-kit/lib/emitter.js:25:14)
    at Emitter.module.exports.Emitter.emit (/Applications/Atom.app/Contents/Resources/app.asar/node_modules/event-kit/lib/emitter.js:129:28)
    at PackageManager.module.exports.PackageManager.emitPackageEvent (/Applications/Atom.app/Contents/Resources/app.asar/node_modules/settings-view/lib/package-manager.js:607:27)
    at /Applications/Atom.app/Contents/Resources/app.asar/node_modules/settings-view/lib/package-manager.js:446:26
    at exit (/Applications/Atom.app/Contents/Resources/app.asar/node_modules/settings-view/lib/package-manager.js:69:16)
    at triggerExitCallback (/Applications/Atom.app/Contents/Resources/app.asar/src/buffered-process.js:303:11)
    at ChildProcess.<anonymous> (/Applications/Atom.app/Contents/Resources/app.asar/src/buffered-process.js:333:11)
    at emitTwo (events.js:106:13)
    at ChildProcess.emit (events.js:191:7)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:215:12)

is this still relevant?

@guncha
Copy link
Contributor Author

guncha commented Mar 2, 2017

@ZuBB it just means that you have both atom-typescript and atom-typescript-beta plugins enabled at the same time and they don't play nicely together. Disabling one or the other will take care of that.

@ZuBB
Copy link

ZuBB commented Mar 3, 2017

@guncha thanks for your reply.

thats OK. I have disabled original version already.

By the way this beta works really quick and smooth. thanks a lot for you efforts 🙇

@ZuBB
Copy link

ZuBB commented Mar 6, 2017

since issues are closed in forked repo ought to do pseudo bug report here

[Enter steps to reproduce:]

1. ...
2. ...

**Atom**: 1.14.4 x64
**Electron**: 1.3.13
**OS**: Mac OS X 10.12.3
**Thrown From**: [atom-typescript-beta](https://github.com/guncha/atom-typescript) package 11.0.1


### Stack Trace

Uncaught TypeError: Cannot read property 'executeGetErr' of undefined

At /Users/vzuzyak/.atom/packages/atom-typescript-beta/dist/main/atomts.js:79

TypeError: Cannot read property 'executeGetErr' of undefined
at Object.lodash_1.debounce (/packages/atom-typescript-beta/dist/main/atomts.js:79:24)
at invokeFunc (/packages/atom-typescript-beta/node_modules/lodash/lodash.js:10350:23)
at trailingEdge (/packages/atom-typescript-beta/node_modules/lodash/lodash.js:10397:18)
at timerExpired (/packages/atom-typescript-beta/node_modules/lodash/lodash.js:10385:18)


### Commands

6x -0:59.1.0 vim-mode-plus:move-to-next-word (input.hidden-input)
9x -0:57.1.0 vim-mode-plus:move-down (input.hidden-input)
-0:54.9.0 vim-mode-plus:move-up (input.hidden-input)
-0:54.7.0 core:save (input.hidden-input)
2x -0:54.2.0 tree-view:toggle (input.hidden-input)
-0:20.2.0 vim-mode-plus:search (input.hidden-input)
-0:18.5.0 core:confirm (input.hidden-input)
3x -0:18 vim-mode-plus:repeat-search (input.hidden-input)
-0:16.1.0 vim-mode-plus:move-to-bottom-of-screen (input.hidden-input)
9x -0:15.7.0 vim-mode-plus:move-down (input.hidden-input)
7x -0:14.1.0 vim-mode-plus:move-up (input.hidden-input)
-0:12.8.0 vim-mode-plus:move-down (input.hidden-input)
-0:12.5.0 vim-mode-plus:move-up (input.hidden-input)
-0:12.2.0 vim-mode-plus:move-to-last-character-of-line (input.hidden-input)
-0:11.5.0 vim-mode-plus:move-left (input.hidden-input)
-0:06.2.0 core:save (input.hidden-input)


### Non-Core Packages

atom-beautify 0.29.17
atom-shortcuts 0.0.2
atom-typescript 10.1.14
atom-typescript-beta 11.0.1
atom-vim-keymap 1.0.1
editor-stats 0.17.0
editorconfig 2.2.2
ex-mode 0.13.1
git-control 0.8.2
git-plus 7.3.3
goto-definition 1.2.0
linter 1.11.23
open-recent 5.0.0
project-manager 3.3.3
svg-preview 0.11.0
sync-settings 0.8.1
vim-mode-plus 0.82.3
vim-mode-plus-ex-mode 0.9.0
webbox-color 0.5.9


I do not have any STR. its just popped out :(

@jimmytheneutrino
Copy link
Contributor

Quickfixes (!!) (these require manipulating the AST that we don’t have direct access to)

Does not tsserver support quickfixes through microsoft/TypeScript#10185?

@guncha
Copy link
Contributor Author

guncha commented Mar 6, 2017

@jimmytheneutrino no reason it couldn't. The Typescript team has added quite a few quickfixes in the last two Typescript versions so this would be a worthwhile feature.

@basarat
Copy link
Member

basarat commented Mar 7, 2017

@guncha Just a heads up. You have full executive authority on atom-typescript and are now an owner under TypeStrong. Make atom-typescript what ever you want! 🌹 ❤️

@guncha
Copy link
Contributor Author

guncha commented Mar 7, 2017

Thanks @basarat! Any ideas how to smoothly transition to the version in this branch?

@basarat
Copy link
Member

basarat commented Mar 7, 2017

Thanks @basarat! Any ideas how to smoothly transition to the version in this branch?

Rename master to legacy and rename this branch to master 🌹 👍

@ZuBB
Copy link

ZuBB commented Mar 13, 2017

I got new error with beta version https://gist.github.com/ZuBB/d35eb7530a142ce3aaf3fc62abc8268e

@guncha
Copy link
Contributor Author

guncha commented Mar 13, 2017

Thanks @ZuBB, it'd be great if you could open an issue in this repo and I'll tag it with beta. It's probably my lazy implementation of the tsserver protocol 😄

@guncha guncha merged commit cc81306 into master Mar 20, 2017
@basarat
Copy link
Member

basarat commented Mar 20, 2017

Wohooo 🌹 Looking forward to installing it ❤🤗

@guncha guncha deleted the use-tsserver branch March 20, 2017 00:53
@guncha
Copy link
Contributor Author

guncha commented Mar 20, 2017

Awesome 😊 I expect to wake up to a million complaints, but we'll get through it..

@aseemk
Copy link

aseemk commented Mar 20, 2017

Wow! Amazing work. =)

@TypeStrong TypeStrong locked and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.