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

wrong indentation with jsx #1019

Closed
22mahmoud opened this issue Mar 12, 2021 · 34 comments · Fixed by #1020
Closed

wrong indentation with jsx #1019

22mahmoud opened this issue Mar 12, 2021 · 34 comments · Fixed by #1020
Labels
bug Something isn't working

Comments

@22mahmoud
Copy link

Describe the bug

I have wrong indentation when hit enter/new line in only in jsx tags

To Reproduce

asciicast
Output of :checkhealth nvim_treesitter

health#nvim_treesitter#check

Installation

  • OK: tree-sitter found 0.18.3(parser generator, used for :TSInstallFromGrammar)
  • OK: git executable found.
  • OK: cc executable found.

Parser/Features H L F I

  • comment ✓ . . .
  • javascript ✓ ✓ ✓ ✓
  • lua ✓ ✓ ✓ ✓
  • c ✓ ✓ ✓ ✓
    Legend: H[ighlight], L[ocals], F[olds], I[ndents]
    *) multiple parsers found, only one will be used
    x) errors found in the query, try to run :TSUpdate {lang}

Output of nvim --version

NVIM v0.5.0-dev+1144-gd38508d88
Build type: RelWithDebInfo
LuaJIT 2.1.0-beta3
Compilation: /usr/bin/cc -O2 -g -Og -g -Wall -Wextra -pedantic -Wno-unused-parameter -Wstrict-prototypes -std=gnu99 -Wshadow -Wconversion -Wmissing-prototypes -Wimplicit-fallthrough -Wvla -fstack-protector-strong -fno-common -fdiagnostics-color=always -DINCLUDE_GENERATED_DECLARATIONS -D_GNU_SOURCE -DNVIM_MSGPACK_HAS_FLOAT32 -DNVIM_UNIBI_HAS_VAR_FROM -DMIN_LOG_LEVEL=3 -I/home//repos/neovim/build/config -I/home//repos/neovim/src -I/home//repos/neovim/.deps/usr/include -I/usr/include -I/home//repos/neovim/build/src/nvim/auto -I/home//repos/neovim/build/include
Compiled by root

Features: +acl +iconv +tui
See ":help feature-compile"

system vimrc file: "$VIM/sysinit.vim"
fall-back for $VIM: "/usr/local/share/nvim"

Run :checkhealth for more info

@22mahmoud 22mahmoud added the bug Something isn't working label Mar 12, 2021
@bluz71
Copy link
Contributor

bluz71 commented Mar 31, 2021

I was going to open a separate issue, but piggybacking here since this is my issue.

Treesitter JSX indentation is a work in progress.

With TS indentation enabled, opening a new line (indentation should be directly under the <li> but is actually back one indentation level):

TS_1st

Then opening up a curly brace is really weird, goes all the way to the first column:

TS_2nd

If I disable TS indentation but still have Treesitter for highlighting, I get these incorrect results (first open newline then add in curly brace):

TS_3rd

TS_4th

If I use Polyglot instead, and disable Treesitter, the indentation works correctly:

Polyglot_1st

Polyglot_2nd

I tried @kyazdani42 fix/indent/new-line-indent branch, but no success due to ABI mismatch. So I am not sure if it works or not for JSX.

If testing needed in future, I would be happy to help.

Cheers.

@bluz71
Copy link
Contributor

bluz71 commented Mar 31, 2021

Question.

Is is possible to use Treesitter for highlighting and Polyglot for indentation? I assume not, but that's kind of what I want whilst Treesitter indentation settles down.

@bluz71
Copy link
Contributor

bluz71 commented Mar 31, 2021

Answering my own question after discussion in this Reddit thread.

No it is not possible to use Polyglot JSX indentation with Treesitter. Polyglot uses vim-jsx-pretty for indentation. And that indentation mechanism uses Vim's tradition syntax subsystem.....which Treesitter explicitly disables. So vim-jsx-pretty indentation will not work whilst Treesitter is in effect.

It is a similar situation with Tim Pope's endwise plugin does not work with Treesitter because syntax is disabled.

The solution will be maturation (better Treesitter indentation) and ecosystem (new Treesitter-aware plugins).

Early adopter pains.

@theHamsta
Copy link
Member

@bluz71 when you're fine with having tree-sitter and syntax running, you can enable them both. There's also an option that nvim-treesitter does not active syntax.

@bluz71
Copy link
Contributor

bluz71 commented Mar 31, 2021

Interesting. My understanding is that Treesitter by default disables syntax (which makes sense to me).

How do I tell Treesitter not to disable syntax? If I do that will it cause highlight issues, which highlighting will be in effect, regex or
treesitter?

@theHamsta
Copy link
Member

Just use additional_vim_regex_highlighting=true in your setup call for the highlight module.

  highlight = {
    module_path = 'nvim-treesitter.highlight',
    enable = false,
    disable = {'markdown'}, -- FIXME(vigoux): markdown highlighting breaks everything for now
    custom_captures = {},
    is_supported = queries.has_highlights,
    additional_vim_regex_highlighting = false,
  },

You can experiment with the effect of this setting, by running :set syntax=1 in a buffer with tree-sitter highlighting. Both tree-sitter and vim syntax will set highlights. Tree-sitter highlights will have priority. You can use this setting as a work-around if you need some syntax feature that is not implemented with tree-sitter yet.

@bluz71
Copy link
Contributor

bluz71 commented Mar 31, 2021

Very interesting. It may well solve my issue. I will experiment with it tomorrow.

I appreciate the tip.

@kyazdani42
Copy link
Member

hi, i have to update my branch but it should fix this indentation issue.

@bluz71
Copy link
Contributor

bluz71 commented Mar 31, 2021

When the branch is ready, let us know, I would like to test it.

@bluz71
Copy link
Contributor

bluz71 commented Apr 1, 2021

@theHamsta,

This configuration works well for me:

local treesitter = require'nvim-treesitter.configs'

treesitter.setup {
  ensure_installed = {
    'c', 'cpp', 'dart', 'go', 'html', 'java', 'javascript', 'python', 'ruby',
    'rust', 'typescript'
  },
  highlight = {
    enable = true,
    additional_vim_regex_highlighting = true
  },
  indent = {
    enable = false
  }
}

additional_vim_regex_highlighting appears to automatically turn on syntax hence I don't need to do anything else.

I get Treesitter highlighting and Polyglot indenting. This is ideal until Treesitter indentation is sorted out, maybe sooner than later via @kyazdani42 branch.

I am pleased. Many thanks.

@bluz71
Copy link
Contributor

bluz71 commented Apr 1, 2021

@kyazdani42's #1020 PR is a massive improvement.

I just did some quick tests and it works great for my simple JavaScript + JSX code. Fantastic.

Note, I am sure there are still some more wrinkles to iron out, but this is a very big improvement. I no longer need to use additional_vim_regex_highlighting, instead I just enable Treesitter indent.

@luisiacc
Copy link

luisiacc commented Apr 2, 2021

@kyazdani42's #1020 PR is a massive improvement.

I just did some quick tests and it works great for my simple JavaScript + JSX code. Fantastic.

Note, I am sure there are still some more wrinkles to iron out, but this is a very big improvement. I no longer need to use additional_vim_regex_highlighting, instead I just enable Treesitter indent.

Do you get jxs indenting correctly?, if so, do you have enabled autoindent, smartindent or filetype indent on or any of those ? Asking because indenting does not seem to work so well for me on jsx files.

@bluz71
Copy link
Contributor

bluz71 commented Apr 3, 2021

I spoke too soon.

{....} indents in JSX work much better than before.

However, tags within tags are still wonky. Apologies for stating all was working well. I do very little JSX so my test was very quick, too quick.

Request indent settings are:

  • autoindent - true
  • smartindent - false (nosmartindent)
  • filetype - javascript

If doing JSX the following settings are probably better for now (aka use vim-jsx-pretty indents via syntax=on):

local treesitter = require'nvim-treesitter.configs'

treesitter.setup {
  ensure_installed = "maintained",
  highlight = {
    enable = true,
    additional_vim_regex_highlighting = true
  },
  indent = {
    enable = false
  }
}

@kyazdani42
Copy link
Member

i do a lot of jsx and haven't noticed wonkiness yet, but i'm still thinking about this indent algorithm, because it's quite hard to do unfortunately due to languages having very specific edge cases. I'd consider indent still WIP.

@bluz71
Copy link
Contributor

bluz71 commented Apr 4, 2021

@kyazdani42 & @luisiacc,

Ok, I have done a heap of testing with Treesitter and JSX; hours of testing.

Treesitter JSX indent is not wonky by itself, in fact it works quite well. My apologies.
Treesitter JSX indent is very wonky if the vim-polyglot plugin is active and installed.

Even disabling Polyglot languages, like the following, is not enough:

let g:polyglot_disabled = ['javascript']

The above g:polyglot_disabled does work for Polyglot commits prior to #2838800. After that commit one needs to comment out the Polyglot plugin altogether (aka, don't load it).

For example, this works quite well (v4.16.0 is prior to the 2838800 commit):

let g:polyglot_disabled = ['javascript']
Plug 'sheerun/vim-polyglot', { 'tag': 'v4.16.0' }

The Treesitter teams needs to be aware the vim-polyglot can badly mess up Treesitter indents. Treesitter will be blamed, but Polyglot is doing the mess up (somehow, which I don't understand).

I am almost of the opinion that the Neovim Treesitter team tell users not to install or use vim-polyglot at all.

I think I am eventually going to stop using Polyglot. For now I will stick with the v4.16.0 Polyglot tag.

Cheers.

@luisiacc
Copy link

luisiacc commented Apr 5, 2021

For me it wasn't working because filetype was set to javascriptreact, if I change to javascript then it works as expected. (I think it should work in both though)

@kyazdani42
Copy link
Member

@luisiacc it should not alter the behavior though, javascript and javascripreact share the same queries.
@bluz71 thanks for the debugging :) i did not see any issues in js/jsx files since the fix was merged.

@luisiacc
Copy link

@luisiacc it should not alter the behavior though, javascript and javascripreact share the same queries.

I'm still having the same problem, indentation on javascriptreact files doesn't indent properly when working with tags, but on javascript filetypes the do work well.

@luisiacc
Copy link

luisiacc commented Jul 6, 2021

@luisiacc it should not alter the behavior though, javascript and javascripreact share the same queries.

I'm still having the same problem, indentation on javascriptreact files doesn't indent properly when working with tags, but on javascript filetypes the do work well.

@kyazdani42 I'm still having this issue on latest nvim-treesitter, trying setting the filetype of the file to javascriptreact and see if you can reproduce.

An example of the cursor would be:

return (
	<Col>
		<Row>
	| <----------- cursor goes here
	</Col>

@kyazdani42
Copy link
Member

indentation is still not perfect. I'm really busy lately so haven't had the time to check nvim-treesitter related issues, because my other plugin got a lot of traction and with work and summer ... I'll take a look at indentation if nobody does it first, but when i'll have some time, so that might not be soon i'm afraid :/

@dalhorinek
Copy link

Just curious if this is still possibly fixed as the issue is closed. Currently the indent still doesn't work as expected. @kyazdani42 Is this still on your roadmap to do? Thank you for your work!

@kyazdani42
Copy link
Member

its not on my roadmap atm, i dont have quite the time :/

@GoldfishPi
Copy link

I wouldn't mind poking at this if you could point me in the right direction @kyazdani42. I use nvim + treesitter for tsx development daily and this has annoyed me for a while so I'm motivated to give it a shot :)

@theHamsta
Copy link
Member

At the moment, indent doesn't support injected languages.
return (


| <----------- cursor goes here

This is because the algorithm only walks the HTML tree until the root and then thinks `<Col>` should be level 0 indent. Also the tree is completely broken at this state. The first issue can be fixed. The second one is more challenging
![image](https://user-images.githubusercontent.com/7189118/144925656-6835a791-d7c5-401b-bd59-b897f73439a1.png)


Ideally, we should also use here the built-in query function of nvim should be used instead of the inefficient nvim-treesitter wrapper https://github.com/theHamsta/nvim-treesitter/blob/de010af2ac59834d89a280e8b56ddf94afcf19bb/lua/nvim-treesitter/indent.lua#L30 and do something in the style of https://github.com/theHamsta/nvim-treesitter/blob/95e56b6936006fc6f2117b06c2fb2f8d422ad65e/tests/query/highlights_spec.lua#L45-L32 to collect the queries of all the lang (with indent queries instead of highlight queries).

I'll have time off starting next Friday. If you want to I can give you some hints on how to you could improve things.

@gmr458
Copy link

gmr458 commented Feb 19, 2022

any update on this?

@dwoznicki
Copy link
Contributor

dwoznicki commented Feb 25, 2022

Hi, I'm not sure if there was a regression at some point, but I'm still getting the bug where adding a "{" in the middle of a JSX block sets the indent to 0.

const Testo = () => {
    return <div>
        {/* adding a newline sets the indentation to here */}
{
|<-- typing a "{" moves the bracket back to here
    </div>
};

Interestingly, when I print out the node type in lua/nvim-treesitter/indent.lua during the while loop in M.get_indent like so:

while node do
  print(node:type())
  " ...
end

I get the following output:

// ---------- adding the newline ----------
>
jsx_opening_element
jsx_element
return_statement
statement_block
arrow_function
variable_declarator
lexical_declaration
program
// ---------- typing the "{" ----------
{
ERROR

Looks kinda similar to #2516, but I'm not sure.

EDIT: Okay, I don't know if it's related, but setting (ERROR) @auto seems to solve this issue. Regular JSX tags also appear to be properly indented with this change as well.

@sahibalejandro
Copy link

Wondering if there is any update on this issue? It still happening to me.

@theHamsta
Copy link
Member

@sahibalejandro I would recommend to set up a test case for your issue and make it fail our CI. If you poke a bit around with indent.scm of ecma,jsx you should be able to fix it. Can you check whether switching the filetype to tsx fixes the issue? What is the file type of the file you're testing on? Is it jsx or javascript? For me it looks like that jsx doesn't include standard Javascript in the query files.

@sahibalejandro
Copy link

@theHamsta the filetype is javascriptreact, I just realized that after doing :set filetype=jsx the indentation behaves correctly. I guess now I just need to figure out how to force the filetype to jsx instead of javascriptreact.

Thanks for the hint tho!

@theHamsta
Copy link
Member

Javascript should contain the Jsx queries. So there are some Js queries that cause wrong indentation

@sahibalejandro
Copy link

sahibalejandro commented Apr 7, 2022

After doing more tests, indentation on jsx and tsx files is not working as expected. Here some results:

  • When opening a .jsx file, Neovim sets the filetype to javascriptreact, I'm not sure which indents query uses treesitter for it.
  • For javascriptreact indentation works well under javascript code, on JSX tags it's broken, in this way:
<SomeComponent>
  <div>
    <div>
      Something here... (new line)
  | <-- cursor jumps here
    </div>
  </div>
</SomeComponent>
  • For filetype of jsx and tsx there are different issues than javascriptreact: Indentation under javascript/typescript code doesn't work:
if (someValidation) {
| <-- cursor jumps here
}
  • For filetype of jsx and tsx indentation under JSX tags is also wrong:
<div>
  <div>
    <span>hello world</span>
  | <-- cursor jumps here
  </div>
</div>

I don't have enough knowledge to write test files in Lua or update the queries, so I can't help with that. Is there any other tests or investigations that I can do to help solve this problem?

Worth to mention that I tested it with a clean Neovim v0.6.1 install with only these plugins: Packer nvim-lspconfig and nvim-treesitter.

@scallaway
Copy link

I've just made the transition from Polyglot to Treesitter, and can confirm that I'm running into at least some variation of this problem.

I can recreate indentation problems with a minimum setup of only treesitter, confirming that I do have the tsx parser installed, looking at a typescriptreact file.

Creating a new line from within a function specified outside the return call in a React component doesn't seem to give it any indentation (like you would expect), and the same goes for adding props or new components within the return call.

This problem seems to be exacerbated once plugins are added that take into account the ESLint file etc, and the new line indentation is way off.

In summary, I'm seeing the same as sahibalejandro for the conditional statements, but it's slightly different for the other scenarios they've specified.

Changing the file type doesn't seem to make any changes for me.

I'm using Nvim 0.7 with the latest treesitter version.

@Coder2195Text
Copy link

Coder2195Text commented Jan 28, 2023

Im also having this issue
image
image

@LAST7
Copy link

LAST7 commented Oct 13, 2023

same issue here :( pretty annoying

Also, the embedded html code in the .jsx file are not highlighted.
However, when it comes to .tsx file, those two problems do not show up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.