typescript-mode with lsp backend and prettier as typescript-fmt-tool doesn't use prettier to format typescript files

NANASHI0X74 commented 4 years ago

Description :octocat:

typescript-mode with lsp backend and prettier typescript-fmt-tool doesn't use prettier to format typescript files. It should be noted that the javascript layer handles this fine (actually what it does is expose keybindings to use either the lsp formatter or prettier).

Reproduction guide :beetle:

Start Emacs
go to a .ts file in a project with .prettierrc.js config
format buffer with SPC-m-=-b

Observed behaviour: :eyes: :broken_heart: lsp-format-buffer is run which doesn't respect .prettierrc and produces conflicts. It does respect .editorconfig though.

Expected behaviour: :heart: :smile: format buffer command runs prettier-js instead of lsp-format-buffer or alternately the layer configures the lsp to respect my .prettierrc ^^

System Info :computer:

OS: gnu/linux
Emacs: 26.3
Spacemacs: 0.300.0
Spacemacs branch: develop (rev. 6145a566e)
Graphic display: t
Distribution: spacemacs
Editing style: vim
Completion: helm

Layers:

(python go
    (typescript :variables typescript-fmt-on-save t typescript-fmt-tool 'prettier typescript-linter 'eslint typescript-lsp-linter nil)
    (javascript :variables javascript-fmt-tool 'prettier javascript-backend 'lsp)
    prettier auto-completion emacs-lisp git helm lsp markdown multiple-cursors org
    (shell :variables shell-default-height 30 shell-default-position 'bottom)
    syntax-checking version-control treemacs)

System configuration features: XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS GLIB NOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM THREADS LIBSYSTEMD LCMS2

Additional info:

this is my .eslintrc.js:

module.exports = {
    env: {
        es6: true,
        node: true,
    },
    extends: [
        'eslint:recommended',
        'plugin:@typescript-eslint/eslint-recommended',
        'plugin:@typescript-eslint/recommended',
        'prettier/@typescript-eslint',
        'plugin:prettier/recommended',
        'prettier',
    ],
};

this is my .prettierrc.js:

module.exports = {
        printWidth: 120,
        // tabWidth: 4, //defined by .editorconfig
        bracketSpacing: true,
        singleQuote: true,
};

and this is my .editorconfig:

[*.{js,ts}]
indent_style = space
indent_size = 4

smile13241324 commented 4 years ago

I think the right thing to do would be to overwrite the standard lsp binding in the layer with a more specific one for the language. PR would be welcome.

NANASHI0X74 commented 4 years ago

so. I just had a look at the package.el and it seems that the typescript layer already binds its own keybinding: https://github.com/syl20bnr/spacemacs/blob/dad4406f70e8d84f6e0eb8dcc1398ddcb05bbaee/layers/%2Blang/typescript/packages.el#L125-L127 It works with tide backend, but when using lsp I assume it gets overwritten by the lsp layer later? Can I somehow ensure that the typescript layer binds its keys after lsp or prevent the lsp layer from binding its keys?

NANASHI0X74 commented 4 years ago

ok, after reading the documentation on spacemacs layers and keybinding functions, I first naively tried to fix it by binding "=" to typescript-format with the following patch:

++ b/layers/+lang/typescript/packages.el                                                                                                                                         
@@ -22,6 +22,7 @@
         web-mode
         tide
         yasnippet
+        lsp-mode
         ))

@@ -136,3 +138,9 @@
   (when (eq (spacemacs//typescript-backend) 'tide)
     (add-to-list 'tide-managed-modes 'typescript-mode)
     (add-to-list 'tide-managed-modes 'typescript-tsx-mode)))
+
+(defun typescript/post-init-lsp-mode ()
+  (spacemacs/set-leader-keys-for-major-mode 'typescript-mode
+    "="  'spacemacs/typescript-format
+    "p" 'spacemacs/typescript-open-region-in-playground))

But upon reloading emacs and opening a typescript file, I couldn't observe any different behavior...

what does instead work though is to bind it to "==" conditionally, if using lsp backend and to "=" otherwise . I'll submit a PR with this fix, but maybe it would be better to bind it to "=" to be consistent with the other backend? If the latter would be preferred, feel free to reject the PR.

Also, could someone educate me on why my first patch wouldn't work? I'm still quite new to emacs/elisp and spacemacs (and haven't done much FP up to now, so having trouble understanding the syntax of the language as well sometimes) and a lot of things about how they work under the hood are still quite opaque to me.

duianto commented 4 years ago

The first PR was against the master branch.

The destination and source are described under the title.

The first PR says: Bind typescript-format to "==" or "=" depending on backend #14048 NANASHI0X74 wants to merge 4,148 commits into syl20bnr:master from NANASHI0X74:patch-ts-format-kbd

The second one PR says: Bind typescript-format to "==" or "=" depending on backend #14049 smile13241324 merged 3 commits into syl20bnr:develop from NANASHI0X74:patch-ts-format-kbd 8 days ago

The contributing pull request instructions say: https://github.com/syl20bnr/spacemacs/blob/develop/CONTRIBUTING.org#pull-request

Submit your contribution against the develop branch. You should not use your master branch to modify Spacemacs, this branch is considered to be read-only.

syl20bnr / spacemacs