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

</> terminating tag [Question/Bug] #1966

Closed
BuGlessRB opened this issue Sep 1, 2016 · 18 comments
Closed

</> terminating tag [Question/Bug] #1966

BuGlessRB opened this issue Sep 1, 2016 · 18 comments
Assignees
Labels

Comments

@BuGlessRB
Copy link

BuGlessRB commented Sep 1, 2016

I created a tag to make the page-title auto-updating, and ended up with this:

<title>
 <yield /> - { title }
 </>
 <script>
  var self = this;
  self.mixin("bus");
  self.bus.on("settitle", function(title)
   { self.update({"title":title});
   });
 </script>
</title>

It turns out , that in order to get this working, I was forced to terminate the HTML part of the tag using </>, just before the <script>. The questions would be:

  1. Is it possible to make that unnecessary by causing the <script> to autoterminate the HTML part instead?
  2. Or if that is not possible, is </> a supported way to accomplish this?

Using <span>...</span> to encapsulate the HTML is not an option, since browsers do not tolerate any nested HTML directives in the <title> tag.

@aMarCruz
Copy link
Contributor

aMarCruz commented Sep 1, 2016

Interesting
@BuGlessRB are you using this tag in <head> ?

This is a compiler's bug.

@aMarCruz aMarCruz self-assigned this Sep 1, 2016
@aMarCruz aMarCruz added the bug label Sep 1, 2016
@BuGlessRB
Copy link
Author

BuGlessRB commented Sep 1, 2016

Yes, just the normal <title> in <head>. Is there any reason why this might not work?
It looks like:

<head>
 <title>SiteName</title>
</head>

And the custom tag turns that into:

<head>
 <title>SiteName - Page Name Foo Bar</title>
</head>

aMarCruz pushed a commit to riot/compiler that referenced this issue Sep 1, 2016
@aMarCruz
Copy link
Contributor

aMarCruz commented Sep 1, 2016

@BuGlessRB this doesn't works because is not ending with a valid closing tag, see the Guide.

With regexes, there's no way to detect expressions ending the html part, so I take your hack to give support for these cases in the compiler with a minor change: this will be <-/> so you can use this in tags inside html pages as well (browsers removes </> before the compiler can read it).

The addition of the BuGless-hack (may I?) is in the "dev" branch and will be release in v2.5.4 of the riot-compiler, and docummented in the section Untagged HTML content of the guide, with a brief example.

@BuGlessRB
Copy link
Author

BuGlessRB commented Sep 1, 2016

The nick is there for a reason, so BuGless-hack is fine :-).

The <-/> solution works for me.
However, I understand that using mere regexes to detect the end of the html part is difficult, but if you use an explicit <script></script>, shouldn't it be trivially possible to detect the start of the <script> part?

@aMarCruz
Copy link
Contributor

aMarCruz commented Sep 1, 2016

nope, you can use multiple script tags + untagged js:

<my-tag>
  var title = 'foo'
  var x = { title }
  <script type="typescript">
    const y :string = "bar"
  </script>
</my-tag>

@BuGlessRB
Copy link
Author

Hmmm, well, that does not strike me as a feature (putting it politely).

@aMarCruz
Copy link
Contributor

aMarCruz commented Sep 1, 2016

agree, but is in the specs before me :)

@BuGlessRB
Copy link
Author

Well, it shouldn't have been in those specs in the first place (if you ask me). So I vote to eliminate that part of the specs for 3.0.0.

I would even go as far as making <script></script> mandatory if you use any script, but would allow for multiple <script></script> blocks.

@aMarCruz
Copy link
Contributor

aMarCruz commented Sep 1, 2016

It was propesed by some people yet, maybe we can do something about this for v3.
Making <script> mandatory solves gotchas and simplifies the code of the compiler, with a safer detection.

@riot/core-maintainers what do you think?

I'm not sure about such change because you can put <script> in riot tags inside html pages.

@BuGlessRB
Copy link
Author

Ah, due to the fact that script tags cannot be nested.
Yes, true.

Nonetheless, I'd say, then make that an exception: i.e. only support missing <script> tags to start the javascript if defined inside a script tag already. If using a normal riottag definition, require a <script> tag to delimit the first use of any scripting language.

@aMarCruz
Copy link
Contributor

aMarCruz commented Sep 1, 2016

mmm... I'll will think about it.

@BuGlessRB
Copy link
Author

Besides solving some parsing issues, I expect for it to speed up the parsing process as well.

Incidentally, I just tried the dev branch from github, but it does not seem to contain the fix just yet. Am I missing something?

koobitor pushed a commit to koobitor/riot that referenced this issue Sep 2, 2016
@GianlucaGuarini
Copy link
Member

I would even go as far as making <script></script> mandatory if you use any script, but would allow for multiple <script></script> blocks.

@aMarCruz @BuGlessRB I would not force our users using scripts to handle their tags logic. Riot is build with the idea of keeping things simple, it's not opinionated so it will give you the freedom to write your code however you want. I wouldn't care that much about such edge cases and protecting users from themselves

@BuGlessRB
Copy link
Author

Well, the only things I can say on that are that:

  • I love the philosophy of Riot to keep things simple.
  • I also wholeheartedly agree with not enforcing certain programming paradigms (i.e. leave the programmer to choose how he solves things).
  • I wholeheartedly disagree that this freedom extends into the used syntax and grammar of a language.
  • Tools to help a programmer do static analysis of code or syntax highlighting work a lot better if the syntax and grammar are well defined.
  • Things like optional delimiters (be it semicolons or <script> tags) I consider a design flaw. They unduly complicate the task of figuring out the intention of the programmer.
  • When enforcing a bit of structure even speeds up the runtime compilation in the browser, then that is considered a free bonus.

@cognitom
Copy link
Member

cognitom commented Sep 3, 2016

</> doesn't make sense for me... It looks not like a part of HTML. And IMO we shouldn't use Riot tag in <head>. That could just introduce a bug into the code.

@aMarCruz honestly speaking, I like the idea of mandatory <script> and believe it's simpler than optional <script>, but it seems a bit inconsistent to the code outside the tag.

theCodeOutsideTagIsAllowedInRiot()
<my-tag>
  ...
</my-tag>

We might need a kind of strict mode but it needs more discussions from many aspects. At least, we shouldn't start the discussion from this issue, I think. Maybe, Riot v4 or later?

@BuGlessRB
Copy link
Author

</> is not meant to make sense, it's just the only way that works to delimit the end of the HTML without using HTML.

Why would using riot tags in things as <title> or <meta name="description"> not be valid and simple use cases?

With regard to mandatory <script>, in inlined tags one could use a nested <riot-script>; and if this discussion should not be held here, then where?

@aMarCruz
Copy link
Contributor

aMarCruz commented Sep 3, 2016

@cognitom mandatory script is good, I use this always, but JS outside the tag seems inconsistent with or without script, I think there's no syntax hilighter that can handle this :) ...anyway, the inability to place nested script tags on pages is the major drawback for implementing this requirement and solutions as riot-script go against the riot philosophy.

Maybe riot v4 can do something about this...
@BuGlessRB I will close this issue now, the bugless-hack is sufficient for cases like this. Thanks!

@aMarCruz aMarCruz closed this as completed Sep 3, 2016
@aMarCruz
Copy link
Contributor

aMarCruz commented Sep 3, 2016

@GianlucaGuarini agreed, thanks.

@BuGlessRB sorry, the update is in the dev branch of the riot-compiler.
Fork the repo, checkout 'dev' and compile this, then link to riot and compile:

git clone https://github.com/riot/compiler && cd compiler
git checkout dev && npm i && make
sudo npm link
cd ../<your riot repo>
npm un riot-compiler && npm link riot-compiler
make riot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants