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.

Sign up for GitHub

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

Jump to bottom

HTML tags are not finding closing tags for longer <div> blocks #522

Closed
pythagorean opened this issue Jul 8, 2019 · 9 comments · Fixed by #523
Closed

HTML tags are not finding closing tags for longer <div> blocks #522

pythagorean opened this issue Jul 8, 2019 · 9 comments · Fixed by #523

Comments

@pythagorean
Copy link

pythagorean commented Jul 8, 2019 •
edited
Loading

Description

I'm submitting a ... (check one with "x")

  • question - please describe the problem you're trying to solve and give as much context as possible.
  • feature request - please describe the behavior you want and the motivation.
    x bug report - please describe shortly your bug and fill out the other sections.

HTML tags are not finding closing tags for longer div blocks.

Expected Results

Successful compilation.

Actual Results

   Compiling coolcats2 v0.1.0 (/Users/mahakal/Development/Apps/coolcats2/ui-src)
error: this open tag has no corresponding close tag
   --> src/application/interfaces/follow.rs:170:13
    |
170 |             <div class="panel panel-default",>
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Context (Environment)

  • Rust: v1.36.0
  • target: wasm32-unknown-unknown
  • cargo-web: v0.6.25
@pythagorean
Copy link
Author

If I remove some of the inner contents of the div block, the code will eventually compile, but of course this is missing part of the html then.

@totorigolo
Copy link
Contributor

Can you post a minimal example?

@pythagorean
Copy link
Author

pythagorean commented Jul 9, 2019 •
edited
Loading

This compiles:

https://gist.github.com/pythagorean/404414b5ae3f14de98a4f180dd1c60d0

By uncommenting the last inner div block, this does not compile, and gives the original error shown:

https://gist.github.com/pythagorean/9b75a25d26ef0d69af5ec470882dc6b7

@totorigolo
Copy link
Contributor

I just tried this in one of my code, and it produces the same error:

html! {
    <div id="buttons">
        <div/>
    </div>
}
error: this open tag has no corresponding close tag
   --> src/lib.rs:264:17
    |
264 |                 <div id="buttons">
    |                 ^^^^^^^^^^^^^^^^^^

So I guess that removing those will compile:

<div class="col-sm-1",/>
<div class="col-sm-4",/>

Then, I would say that the problem is with mixing self-closing and not-self-closing tags.

@totorigolo
Copy link
Contributor

totorigolo commented Jul 9, 2019 •
edited
Loading

Indeed, the html! macro doesn't really consider / in tags:

https://github.com/DenisKolodin/yew/blob/a33a89f0f185a157b4440ad82eef66daf5f98896/crates/macro-impl/src/html_tree/html_tag/mod.rs#L188-L194

https://github.com/DenisKolodin/yew/blob/a33a89f0f185a157b4440ad82eef66daf5f98896/crates/macro-impl/src/html_tree/html_tag/mod.rs#L245-L250

EDIT: Nevermind, it does:
https://github.com/DenisKolodin/yew/blob/a33a89f0f185a157b4440ad82eef66daf5f98896/crates/macro-impl/src/html_tree/html_tag/mod.rs#L162

I think that the problem is because HtmlTagOpen::peek/verify_end doesn't check for the div.

totorigolo added a commit to totorigolo/yew that referenced this issue Jul 9, 2019
@totorigolo
html!: fix mixed self-closing and non-sc tags
7e92891 
Before, self-closing tags where considered as open tags in `verify_end`,
causing codes like this to not compile:

```rust
html! {
    <div>
        <div/> // <- considered as open tag
    </div>
}
```

```
error: this open tag has no corresponding close tag
   --> src/lib.rs:264:17
    |
... | <div>
    | ^^^^^
```

However, this fix isn't ideal because it peeks the buffer twice for
non self-closing tags. I did it that way in order to keep the peek
thing. An alternative would be to turn HtmlSelfClosingTag::peek into a
standard function returning (ident, is_self_closing).

Fixes: yewstack#522
@totorigolo totorigolo mentioned this issue Jul 9, 2019
Merged
@totorigolo
Copy link
Contributor

@pythagorean Can you try this PR and tell me if it fixes this issue? 🙂

@totorigolo
Copy link
Contributor

https://stackoverflow.com/a/3558200/2209243

Using self-closing div tags is invalid HTML5, so I closed my PR. You should use an empty pair instead (i.e. <div props="..."></div>).

@pythagorean
Copy link
Author

@pythagorean Can you try this PR and tell me if it fixes this issue? 🙂

Fixes.

@jstarry jstarry reopened this Jul 21, 2019
@jstarry
Copy link
Member

jstarry commented Jul 21, 2019

JSX supports this, we should too. See here: https://reactjs.org/docs/jsx-in-depth.html

totorigolo added a commit to totorigolo/yew that referenced this issue Jul 23, 2019
@totorigolo
html!: allow mixing self-closing and non-sc tags
59e90a9 
Self-closing tags are more convenient when there are no children. For
instance, React's JSX allows it.

Before, self-closing tags where considered as open tags in `verify_end`,
causing codes like this to not compile:

```rust
html! {
    <div>
        <div/> // <- considered as open tag
    </div>
}
```

```
error: this open tag has no corresponding close tag
   --> src/lib.rs:264:17
    |
... | <div>
    | ^^^^^
```

Add a new `Peek`-able `HtmlSelfClosingTag`, used in `verify_end`.

However, this fix isn't ideal because it peeks the buffer twice for
non self-closing tags. I did it that way in order to keep the peek
thing. An alternative would be to turn HtmlSelfClosingTag::peek into a
function returning (ident, is_self_closing).

Fixes: yewstack#522
totorigolo added a commit to totorigolo/yew that referenced this issue Jul 23, 2019
@totorigolo
html!: allow mixing self-closing and non-sc tags
57ffe3e 
Self-closing tags are more convenient when there are no children. For
instance, React's JSX allows it.

Before, self-closing tags where considered as open tags in `verify_end`,
causing codes like this to not compile:

```rust
html! {
    <div>
        <div/> // <- considered as open tag
    </div>
}
```

```
error: this open tag has no corresponding close tag
   --> src/lib.rs:264:17
    |
... | <div>
    | ^^^^^
```

Add a new `Peek`-able `HtmlSelfClosingTag`, used in `verify_end`.

However, this fix isn't ideal because it peeks the buffer twice for
non self-closing tags. I did it that way in order to keep the peek
thing. An alternative would be to turn HtmlSelfClosingTag::peek into a
function returning (ident, is_self_closing).

Fixes: yewstack#522
totorigolo added a commit to totorigolo/yew that referenced this issue Jul 25, 2019
@totorigolo
html!: allow mixing self-closing and non-sc tags
892856d 
Self-closing tags are more convenient when there are no children. For
instance, React's JSX allows it.

Before, self-closing tags where considered as open tags in `verify_end`,
causing codes like this to not compile:

```rust
html! {
    <div>
        <div/> // <- considered as open tag
    </div>
}
```

```
error: this open tag has no corresponding close tag
   --> src/lib.rs:264:17
    |
... | <div>
    | ^^^^^
```

Add a new `Peek`-able `HtmlSelfClosingTag`, used in `verify_end`.

However, this fix isn't ideal because it peeks the buffer twice for
non self-closing tags. I did it that way in order to keep the peek
thing. An alternative would be to turn HtmlSelfClosingTag::peek into a
function returning (ident, is_self_closing).

I added a TODO for when proc-macro::Diagnostic gets stabilized, in
order to replace the clunky `eprintln!` with proper diagnostics. I
didn't try to return a Result to get a nice error right now because
this error should be fairly rare so we can just wait IMO.

Fixes: yewstack#522
totorigolo added a commit to totorigolo/yew that referenced this issue Jul 25, 2019
@totorigolo
html!: allow mixing self-closing and non-sc tags
dd54b18 
#### Problem
Self-closing tags are more convenient when there are no children. For
instance, React's JSX allows it.

Before, self-closing tags where considered as open tags in `verify_end`,
causing codes like this to not compile:

```rust
html! {
    <div>
        <div/> // <- considered as open tag
    </div>
}
```

```
error: this open tag has no corresponding close tag
   --> src/lib.rs:264:17
    |
... | <div>
    | ^^^^^
```

#### Solution
Add a new `Peek`-able `HtmlSelfClosingTag`, used in `verify_end`.

However, this fix isn't ideal because it peeks the buffer twice for
non self-closing tags. I did it that way in order to keep the peek
thing. An alternative would be to turn HtmlSelfClosingTag::peek into a
function returning (ident, is_self_closing).

#### Notes
I added a TODO for when proc-macro::Diagnostic gets stabilized, in
order to replace the clunky `eprintln!` with proper diagnostics. I
didn't try to return a Result to get a nice error right now because
this error should be fairly rare so we can just wait IMO.

Fixes: yewstack#522
totorigolo added a commit to totorigolo/yew that referenced this issue Jul 26, 2019
@totorigolo
html!: allow mixing self-closing and non-sc tags
5908d33 
#### Problem
Self-closing tags are more convenient when there are no children. For
instance, React's JSX allows it.

Before, self-closing tags where considered as open tags in `verify_end`,
causing codes like this to not compile:

```rust
html! {
    <div>
        <div/> // <- considered as open tag
    </div>
}
```

```
error: this open tag has no corresponding close tag
   --> src/lib.rs:264:17
    |
... | <div>
    | ^^^^^
```

#### Solution
Add a new `Peek`-able `HtmlSelfClosingTag`, used in `verify_end`.

However, this fix isn't ideal because it peeks the buffer twice for
non self-closing tags. I did it that way in order to keep the peek
thing. An alternative would be to turn HtmlSelfClosingTag::peek into a
function returning (ident, is_self_closing).

#### Notes
I added a TODO for when proc-macro::Diagnostic gets stabilized, in
order to replace the clunky `eprintln!` with proper diagnostics. I
didn't try to return a Result to get a nice error right now because
this error should be fairly rare so we can just wait IMO.

Fixes: yewstack#522
bors bot added a commit that referenced this issue Jul 26, 2019
@bors @totorigolo
Merge #523
5e6a8af 
523: html!: fix mixed self-closing and non-sc tags r=jstarry a=totorigolo

Before, self-closing tags where considered as open tags in `verify_end`, causing codes like this to not compile:

```rust
html! {
    <div>
        <div/> // <- considered as open tag
    </div>
}
```

```
error: this open tag has no corresponding close tag
   --> src/lib.rs:264:17
    |
... | <div>
    | ^^^^^
```

However, this fix isn't ideal because it peeks the buffer twice for non self-closing tags. I did it that way in order to keep the peek thing. An alternative would be to turn `HtmlSelfClosingTag::peek` into a standard function returning `(ident, is_self_closing)`.

Fixes: #522

Co-authored-by: Thomas Lacroix <toto.rigolo@free.fr>
@bors bors bot closed this as completed in #523 Jul 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

html!: fix mixed self-closing and non-sc tags totorigolo/yew
3 participants
@jstarry @totorigolo @pythagorean