Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

improve typescript tsx support #8387

Open
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
Contributor

ksjogo commented Feb 16, 2017

No description provided.

Contributor

aaronjensen commented Mar 17, 2017

Awesome work! I found an issue, it looks like if the first and only file you open is a tsx file the tsserver is not started which causes company-tide to not work.

Contributor

ksjogo commented Mar 20, 2017

Do you see any warning or error in the messages buffer?

Contributor

aaronjensen commented Mar 20, 2017

No, but I think the issue is that the hook to start the server maybe isn't added until after the typescript package is loaded which doesn't happen until the first file is open. So it's not actually tsx that's the problem, it's the first ts or tsx file after starting emacs.

layers/+lang/typescript/packages.el
+(defun typescript/post-init-rainbow-identifiers ()
+ (add-to-list 'rainbow-identifiers-faces-to-override 'web-mode-variable-name-face)
+ (typescript||iterate-modes
+ (add-hook 'hook 'rainbow-identifiers-mode)))
@aaronjensen

aaronjensen Mar 20, 2017

Contributor

👎 This enables rainbow identifiers in configurations that do not have it or want it.

+ :post-config
+ (typescript||iterate-modes
+ (flycheck-add-mode 'typescript-tide 'mode)
+ (flycheck-add-mode 'typescript-tslint 'mode)
@aaronjensen

aaronjensen Mar 20, 2017

Contributor

I haven't tested it yet, but this might also run too late to get added to the first file. Is there a reason this got added to tide's post-config hook?

+ "rr" 'tide-rename-symbol
+ "Sr" 'tide-restart-server)
+
+ (add-hook 'hook 'tide-setup)))))
@aaronjensen

aaronjensen Mar 20, 2017

Contributor

this needs to go back in :init so that the first file opened will start tide.

Owner

syl20bnr commented May 15, 2017 edited

@ksjogo Did you fix the review comments from @aaronjensen ?

Contributor

ksjogo commented May 17, 2017

@syl20bnr The ordering was improved. Any recommendations on adding the rainbow-identifiers support? Should that be moved to that layer?

Contributor

aaronjensen commented May 17, 2017

@ksjogo The layer still does not work for the first file opened.

  • If it is a ts file, flycheck-mode is not enabled in the first file opened.
  • If it is a tsx file, neither tide-mode nor flycheck-mode is enabled.

To test, restart emacs and open a ts or tsx file.

Contributor

ksjogo commented May 17, 2017

The pull request did not pick up the latest changes. Just pushed again.

Contributor

aaronjensen commented May 17, 2017

The pull request did not pick up the latest changes. Just pushed again.

Sorry, it is still the same result for me.

Contributor

ksjogo commented May 18, 2017

I am editing tsx? files daily with it so am a bit surprised about that. What's your calculated load order of packages?

Contributor

aaronjensen commented May 18, 2017

The problem is not w/ editing them daily, it is with the very first tsx file that is loaded after emacs is started. Try SPC q R to restart w/o restoring layouts, then open a tsx file.

How do I tell you my calculated load order of packages? My package-selected-packages is:

package-selected-packages
org-capture-pop-frame
company-lua
ghub+
apiwrap
ghub
mu4e-maildirs-extension
mu4e-alert
gmail-message-mode
ham-mode
html-to-markdown
flymd
edit-server
phpunit
phpcbf
php-extras
php-auto-yasnippets
drupal-mode
php-mode
swift-mode
counsel-gtags
esup
nginx-mode
string-inflection
symon
xclip
browse-at-remote
lua-mode
gnuplot-mode
wgrep
smex
rjsx-mode
ivy-purpose
ivy-hydra
flyspell-correct-ivy
counsel-dash
org-mobile-sync
shift-number
eslintd-fix
vmd-mode
request-deferred
deferred
org-gcal
solarized-theme
fuzzy
magithub
flycheck-dogma
flycheck-dialyxir
pdf-tools
winum
counsel-projectile
counsel
unfill
wgrep-ag
eros
vimrc-mode
dactyl-mode
nameless
evil-multiedit
add-node-modules-path
org-tree-slide
ox-reveal
restclient-helm
ob-restclient
company-restclient
know-your-http-well
hide-comnt
package-lint
ob-elixir
helm-purpose
window-purpose
imenu-list
minitest
pug-mode
tide
typescript-mode
restclient
ob-http
zoutline
parent-mode
goto-chg
undo-tree
diminish
flx
seq
spinner
bind-key
pkg-info
epl
flycheck-credo
flycheck-package
osx-dictionary
company-flow
dumb-jump
ht
flycheck-flow
helm-gtags
ggtags
emoji-cheat-sheet-plus
editorconfig
company-emoji
org
marshal
flycheck-mix
evil-unimpaired
popup
evil-terminal-cursor-changer
org-projectile
mwim
github-search
flycheck-elm
elm-mode
yaml-mode
dockerfile-mode
docker
tablist
docker-tramp
flyspell-correct-helm
anzu
highlight
ox-gfm
color-identifiers-mode
flyspell-correct
align-cljlet
iedit
nlinum-relative
nlinum
bind-map
dash
evil-visual-mark-mode
ruby-end
s
ivy
async
hydra
org-download
projectile
smartparens
helm
helm-core
avy
package-build
evil
eyebrowse
column-enforce-mode
clojure-snippets
clj-refactor
inflections
edn
peg
cider-eval-sexp-fu
cider
queue
clojure-mode
evil-cleverparens
paredit
xterm-color
web-mode
web-beautify
toc-org
tagedit
stickyfunc-enhance
srefactor
spaceline
powerline
smeargle
slim-mode
shell-pop
shackle
scss-mode
sass-mode
rvm
ruby-tools
ruby-test-mode
rubocop
rspec-mode
robe
reveal-in-osx-finder
rcirc-notify
rcirc-color
rbenv
ranger
rake
rainbow-mode
rainbow-identifiers
pbcopy
osx-trash
orgit
org-repo-todo
org-present
org-pomodoro
alert
log4e
gntp
org-plus-contrib
org-bullets
multi-term
mmm-mode
markdown-toc
markdown-mode
magit-gitflow
magit-gh-pulls
macrostep
livid-mode
skewer-mode
simple-httpd
lispy
swiper
less-css-mode
launchctl
json-mode
json-snatcher
json-reformat
js2-refactor
multiple-cursors
js2-mode
js-doc
jade-mode
htmlize
helm-gitignore
request
helm-flyspell
helm-dash
helm-css-scss
helm-company
helm-c-yasnippet
haml-mode
graphviz-dot-mode
gnuplot
gitignore-mode
github-clone
github-browse-file
gitconfig-mode
gitattributes-mode
git-timemachine
git-messenger
git-link
git-gutter-fringe+
git-gutter-fringe
fringe-helper
git-gutter+
git-gutter
gist
gh
logito
pcache
gh-md
flycheck-pos-tip
flycheck
floobits
evil-magit
magit
magit-popup
git-commit
with-editor
eshell-z
eshell-prompt-extras
esh-help
erlang
emmet-mode
elisp-slime-nav
dtrt-indent
diff-hl
deft
dash-at-point
company-web
web-completion-data
company-tern
dash-functional
tern
company-statistics
company-quickhelp
pos-tip
company-flx
coffee-mode
chruby
bundler
inf-ruby
auto-yasnippet
yasnippet
auto-dictionary
auto-compile
packed
alchemist
company
elixir-mode
ac-ispell
auto-complete
ws-butler
window-numbering
which-key
volatile-highlights
vi-tilde-fringe
uuidgen
use-package
spacemacs-theme
smooth-scrolling
restart-emacs
rainbow-delimiters
quelpa
popwin
persp-mode
pcre2el
paradox
page-break-lines
open-junk-file
neotree
move-text
monokai-theme
lorem-ipsum
linum-relative
link-hint
leuven-theme
info+
indent-guide
ido-vertical-mode
hungry-delete
hl-todo
highlight-parentheses
highlight-numbers
highlight-indentation
help-fns+
helm-themes
helm-swoop
helm-projectile
helm-mode-manager
helm-make
helm-flx
helm-descbinds
helm-ag
google-translate
golden-ratio
flx-ido
fill-column-indicator
fancy-battery
f
expand-region
exec-path-from-shell
evil-visualstar
evil-tutor
evil-surround
evil-search-highlight-persist
evil-numbers
evil-nerd-commenter
evil-mc
evil-matchit
evil-lisp-state
evil-indent-plus
evil-iedit-state
evil-exchange
evil-escape
evil-ediff
evil-args
evil-anzu
eval-sexp-fu
define-word
clean-aindent-mode
buffer-move
bracketed-paste
auto-highlight-symbol
aggressive-indent
adaptive-wrap
ace-window
ace-link
ace-jump-helm-line
Contributor

ksjogo commented May 18, 2017

Indeed, but using it often means I often open first files and have flycheck and tide running for these buffers.

Contributor

aaronjensen commented May 18, 2017

Here's some debug info:

./*Messages*:158:XXX LOADED flycheck              
./*Messages*:159:XXX typescript/post-init-flycheck
./*Messages*:161:XXX typescript/post-init-tide    
>> Flex.tsx has auto save data; consider M-x recover-this-file
./*Messages*:183:XXX LOADED web-mode              
./*Messages*:186:XXX LOADED tide                  
./*Messages*:187:XXX typescript/init-tide CONFIG  
./*Messages*:188:XXX ADDING TIDE HOOK [2 times]   
./*Messages*:189:XXX ADDING FLYCHECK HOOKS        

The >> is the beginning of opening the first buffer. All of the hooks get added after this, which means they won't get run as far as I can tell.

Contributor

aaronjensen commented May 18, 2017

My modified packages.el so you can see where I put messages:

;;; packages.el --- typescript Layer packages File for Spacemacs
;;
;; Copyright (c) 2012-2017 Sylvain Benner & Contributors
;;
;; Author: Chris Bowdon <c.bowdon@bath.edu>
;; URL: https://github.com/syl20bnr/spacemacs
;;
;; This file is not part of GNU Emacs.
;;
;;; License: GPLv3

(setq typescript-packages
      '(
        company
        eldoc
        flycheck
        rainbow-identifiers
        tide
        typescript-mode
        web-mode
        ))

(defmacro typescript||iterate-modes (&rest body)
  `(progn
     ,@(loop for mode in (if typescript-use-web-mode-for-ts '(web-typescript-mode) '(typescript-mode web-typescript-mode))
             collect (let ((hook (intern (format "%S-hook" mode)))
                           (jump-handlers (intern (concat "spacemacs-jump-handlers-" (symbol-name mode)))))
                       `(progn
                          ,@(subst mode 'mode (subst hook 'hook (subst jump-handlers 'jump-handlers body))))))))

(defun typescript/post-init-company ()
  (when (configuration-layer/package-usedp 'tide)
    (typescript||iterate-modes
     (spacemacs|add-company-backends
       :backends company-tide
       :modes mode))))

(defun typescript/post-init-rainbow-identifiers ()
  (when (or (eq colors-colorize-identifiers 'all) (eq colors-colorize-identifiers 'variables))
    (add-to-list 'rainbow-identifiers-faces-to-override 'web-mode-variable-name-face)
    (typescript||iterate-modes
     (add-hook 'hook 'rainbow-identifiers-mode))))

(defun typescript/post-init-eldoc ()
  (typescript||iterate-modes
   (add-hook 'hook 'eldoc-mode)))

(defun typescript/post-init-flycheck ()
  (message "XXX typescript/post-init-flycheck")
  (spacemacs|use-package-add-hook tide
    :post-config
    (message "XXX ADDING FLYCHECK HOOKS")
    (typescript||iterate-modes
     (flycheck-add-mode 'typescript-tide 'mode)
     (flycheck-add-mode 'typescript-tslint 'mode)
     (spacemacs/enable-flycheck 'mode))))

(with-eval-after-load 'tide
  (message "XXX LOADED tide"))
(with-eval-after-load 'flycheck
  (message "XXX LOADED flycheck"))
(with-eval-after-load 'web-mode
  (message "XXX LOADED web-mode"))

(defun typescript/init-tide ()
  (use-package tide
    :defer t
    :commands (typescript/jump-to-type-def)
    :init
    (progn
      (evilified-state-evilify tide-references-mode tide-references-mode-map
        (kbd "C-k") 'tide-find-previous-reference
        (kbd "C-j") 'tide-find-next-reference
        (kbd "C-l") 'tide-goto-reference))
    :config
    (message "XXX typescript/init-tide CONFIG")
    (progn
      (defun typescript/jump-to-type-def()
        (interactive)
        (tide-jump-to-definition t))

      (typescript||iterate-modes
       (spacemacs/declare-prefix-for-mode 'mode "mg" "goto")
       (spacemacs/declare-prefix-for-mode 'mode "mh" "help")
       (spacemacs/declare-prefix-for-mode 'mode "mn" "name")
       (spacemacs/declare-prefix-for-mode 'mode "mr" "rename")
       (spacemacs/declare-prefix-for-mode 'mode "mS" "server")
       (spacemacs/declare-prefix-for-mode 'mode "ms" "send")

       (spacemacs/set-leader-keys-for-major-mode 'mode
         "gb" 'tide-jump-back
         "gt" 'typescript/jump-to-type-def
         "gu" 'tide-references
         "hh" 'tide-documentation-at-point
         "rr" 'tide-rename-symbol
         "Sr" 'tide-restart-server)

       (message "XXX ADDING TIDE HOOK")
       (add-hook 'hook 'tide-setup)))))

(defun typescript/post-init-tide ()
  (message "XXX typescript/post-init-tide")
  (setf (flycheck-checker-get 'typescript-tide 'modes) '(typescript-mode web-typescript-mode))
  (typescript||iterate-modes
   (spacemacs|define-jump-handlers mode)
   (add-to-list 'jump-handlers 'tide-jump-to-definition)))

(defun typescript--prettify-symbols ()
  (push '("function" . ?λ) prettify-symbols-alist)
  (push '("React.createElement" . ?Ʀ) prettify-symbols-alist)
  (push '("return" . ?⏎) prettify-symbols-alist)
  (push '("async." . ?Թ) prettify-symbols-alist)
  (push '("this.props.store." . ?Σ) prettify-symbols-alist)

  (prettify-symbols-mode))

;; we need to (re)use the init-web-mode here as our mode needs to be defined before the misc post-init functions
(defun typescript/init-web-mode ()
  (define-derived-mode web-typescript-mode web-mode "web-typescript")
  (typescript||iterate-modes
   (add-hook 'hook 'typescript--prettify-symbols)
   (add-hook 'hook (lambda () (yas-minor-mode -1)))
   (when typescript-fmt-on-save
     (add-hook 'hook 'typescript/fmt-before-save-hook))
   (spacemacs/set-leader-keys-for-major-mode 'mode
     "="  'typescript/format
     "sp" 'typescript/open-region-in-playground)))

(defun typescript/post-init-web-mode ()
  (when typescript-use-web-mode-for-ts
    (add-to-list 'auto-mode-alist '("\\.ts\\'" . web-typescript-mode)))
  (add-to-list 'auto-mode-alist '("\\.tsx\\'" . web-typescript-mode)))

(defun typescript/init-typescript-mode ()
    (use-package typescript-mode
      :defer t))
+(defun typescript/init-web-mode ()
+ (define-derived-mode web-typescript-mode web-mode "web-typescript")
+ (typescript||iterate-modes
+ (add-hook 'hook 'typescript--prettify-symbols)
@aaronjensen

aaronjensen May 18, 2017

Contributor

Please add a configuration variable for this, it's likely to be divisive.

Contributor

Kethku commented Jul 16, 2017

I'm looking to make some improvements to the typescript layer as well. I'm not sure how I should proceed since this looks to have been abandoned. Should I make my own fork with the changes above and attempt the fixes that aaronjensen suggested? I'm not super familiar with github procedure...

Contributor

ksjogo commented Jul 17, 2017

@Kethku Added you to my repo. Feel free to add commits to the develop branch, they should show up here automatically.

Contributor

Kethku commented Jul 17, 2017

Awesome. I will take a look at this sometime this week

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