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

Emmet not working in JSX statement when condition contains less than #55411

Closed
mathdeziel opened this issue Jul 31, 2018 · 11 comments
Closed

Emmet not working in JSX statement when condition contains less than #55411

mathdeziel opened this issue Jul 31, 2018 · 11 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug emmet Emmet related issues verified Verification succeeded
Milestone

Comments

@mathdeziel
Copy link
Contributor

mathdeziel commented Jul 31, 2018

  • VSCode Version: 1.25.1 (also tried w/ 1.26.0-insider)
  • OS Version: Windows_NT ia32 10.0.17134

I was going through an intro course to React and stumbled on some issues with Emmet. After reading the documentation, I got it to work for the most part, but I couldn't find anything regarding to my issue.

There seems to be a bug with emmet in else if blocks with JSX in .JS and .JSX files.


Edit: A member of the community pointed out that the issue was related to the less than sign (<) and not to the else if and so the issue was renamed. Pleased read through the other comments for more information.


In short, the tab completion will work in a return if it is located in an if or an else, but will not work if it is located in an else if block (please refer to the GIF below).

Steps to Reproduce:

  1. Create a temp folder for the project
  2. Add the emmet config provided below to the User Settings or Workspace Settings
  3. Add a new file named List.js and populate it with the content provided below
  4. Locate the method renderChangePercent around line 19 of the file, the comments will show where emmet works and where is located the bug.

Here is a GIF of the issue :
vscode_emmet_jsx_elseif

Here is my emmet config :

{
    "emmet.includeLanguages": {
        "javascript": "javascriptreact"
    },
    "emmet.triggerExpansionOnTab": true
}

And here is the file for testing :

import React from 'react';

class List extends React.Component {
    constructor() {
        super();

        this.state = {
            loading: false,
            currencies: [],
            error: null,
        };
    }

    componentDidMount() {
        this.setState({loading: true});
    }
    

    renderChangePercent(percent) {
        if (percent > 0) {
            return <span className="percent-raised">{percent}% &uarr;</span>
        } 
        // Emmet does not work (tabs)
        else if (percent < 0) {
            return <div></div> 
        }
        // Emmet works
        else {
            return <span className="itWorks"></span>
        }
    }

    render() {
        if (this.state.loading) {
            return <div>Loading...</div>
        }
    }
}

export default List;
@vscodebot vscodebot bot added the insiders label Jul 31, 2018
@ramya-rao-a ramya-rao-a added emmet Emmet related issues and removed insiders labels Aug 1, 2018
@ramya-rao-a ramya-rao-a added this to the August 2018 milestone Aug 1, 2018
@ramya-rao-a ramya-rao-a added the bug Issue identified by VS Code Team member as probable bug label Aug 1, 2018
@ramya-rao-a
Copy link
Contributor

Thanks for reporting @mathdeziel

@JacksonKearl Can you take a look at this next week? https://github.com/Microsoft/vscode/blob/master/extensions/emmet/src/defaultCompletionProvider.ts would be the place to start.

@JacksonKearl
Copy link
Contributor

Looks like the real culprit is the <.

0 < 1;
sp // wont trigger
1 > 0;
sp // will trigger

@mathdeziel mathdeziel changed the title Emmet not working in Else If blocks with JSX Emmet not working in JSX statement when condition contains less than Aug 2, 2018
@mathdeziel
Copy link
Contributor Author

Good catch @JacksonKearl, you're right! I should've tested that...

Here is a GIF attesting this hypothesis (PS: don't look at the typo in the comment...)
vscode_emmet_jsx_lessthan

I also retitled the issue to better reflect the real problem.

Thanks!

@JacksonKearl
Copy link
Contributor

This was likely caused by #35128, where we wanted to disable emmet inside tags:

<img sr| // emmet shouldnt trigger here

I'll have to think about what the best solution here is. An obvious one would be to disable backtracking to look for opening brackets once you reach some set of characters (maybe semicolon, end paren, or newline?). But I'm not convinced that's the right fix.

@ramya-rao-a
Copy link
Contributor

ramya-rao-a commented Aug 3, 2018

When backtracking, if we reach < but the character right after that cannot be part of a valid tag name, then we can ignore it. So the change can go here: https://github.com/Microsoft/vscode/blob/c3ad9789820afc6f9cab985bfa6e48175e414499/extensions/emmet/src/abbreviationActions.ts#L497 which already ignores the case of <? (found in php)

@JacksonKearl
Copy link
Contributor

JacksonKearl commented Aug 3, 2018

But in this case, the character after is a space, which is valid as a tag too

@ramya-rao-a
Copy link
Contributor

space is not valid in a tag name as far as I know...

@mathdeziel
Copy link
Contributor Author

Based on the W3C recommendation, spaces are valid in a tag name but only after the tag name itself, even if there are no attributes.

From my understanding,

<tag>            // valid
<tag >           // valid
<tag src="">     // valid
<  tag>          // invalid

So I would guess @ramya-rao-a's solution would work.

@JacksonKearl
Copy link
Contributor

Looks like you’re right. I thought
< tag/> could be valid because we syntax hilight it as a normal tag.

@mathdeziel
Copy link
Contributor Author

I took the liberty to create a PR since it seemed to be an easy fix for a first OSS contribution with the help of @ramya-rao-a's recommendations -- which were spot on 😉 .

@JacksonKearl JacksonKearl removed their assignment Aug 4, 2018
@mathdeziel
Copy link
Contributor Author

mathdeziel commented Aug 5, 2018

PR has been merged, closing the issue.

@roblourens roblourens added the verified Verification succeeded label Aug 29, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Sep 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug emmet Emmet related issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

4 participants