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

__checked vs checked? – make syntax without compiler feel friendlier #276

Closed
timoxley opened this issue Feb 1, 2015 · 22 comments
Closed

__checked vs checked? – make syntax without compiler feel friendlier #276

timoxley opened this issue Feb 1, 2015 · 22 comments
Assignees
Milestone

Comments

@timoxley
Copy link

@timoxley timoxley commented Feb 1, 2015

the current __ prefix on boolean attributes makes it feel like I'm a bad person and I'm doing something wrong by opting out of the compile step.

Perhaps the Boolean attribute syntax could be altered to be more friendly and descriptive e.g. append a question-mark instead: checked?.

@timoxley timoxley changed the title __checked vs checked? – make non-compiled use feel less dirty __checked vs checked? – make syntax without compiler feel friendlier Feb 1, 2015
@tipiirai
Copy link
Contributor

@tipiirai tipiirai commented Feb 3, 2015

I like question mark. Any chance to try whether this special character works consistently across browsers? IE8 in particular.

@tipiirai
Copy link
Contributor

@tipiirai tipiirai commented Feb 10, 2015

Curious: why are you not using the compiler?

I'm thinking of deprecating riot.tag, but I first want to know if there are some serious use cases I'm not aware of.

@timoxley
Copy link
Author

@timoxley timoxley commented Feb 12, 2015

@tipiirai With frameworks coming in and out of vogue every 18 months or so, developers need to be diligent, avoiding the temptation to drink the kool-aid and be constantly planning to eject their existing toolchain. Having a custom DSL is a great way to keep developers locked into your framework, but not necessarily by choice.

I'm thinking of deprecating riot.tag

That's very imposing. riot.tag is how I keep the Riot lock-in out of my code. The reason Riot initially seemed appealing was that it seemed to be rather humble, embracing the idea that it's just a piece of the puzzle, rather than the usual babysitter-with-delusions-of-grandeur-now-with-bonus kitchen-sink framework. I don't want to change how my app is structured, or how I write my code, or have to install a custom syntax highlighter just to use your tool. I just want to use your API. In other words, using a custom DSL for every tool is too invasive and requires far too much buy in.

@tipiirai
Copy link
Contributor

@tipiirai tipiirai commented Feb 13, 2015

Thanks. Yeah, seems many people want to use riot.tag so I'm keeping it there. This article also convinced me.

Did you try the "?"- mark on IE8? This lovely browser is the sole reason for the __ syntax because it looses the expression value on boolean attributes and always converts them to true.

@atian25
Copy link
Contributor

@atian25 atian25 commented Feb 15, 2015

+1 for no deprecating riot.tag.

can we just has a config to __ ? some developer like me will never support ie8 anymore.

@tipiirai
Copy link
Contributor

@tipiirai tipiirai commented Feb 16, 2015

Looks like "?" character works on the attribute name across browsers. Here is my small test page for it.

Leaving out a special character for IE8+ means that I have to detect whether the attribute is boolean on the client side and I don't know if there are any other ways than importing the big list of boolean attribute names to the client. Trying to avoid that. Later perhaps, I can list the most popular ones and the user can add elements to that list manually.

@tipiirai
Copy link
Contributor

@tipiirai tipiirai commented Feb 16, 2015

Going to switch to "?" character on v2.1.0 since this is a backwards incompatible change.

@atian25
Copy link
Contributor

@atian25 atian25 commented Feb 25, 2015

why not use the same syntax as riot-src / riot-style, just use riot-checked.

btw, rt-src is short~

@tipiirai
Copy link
Contributor

@tipiirai tipiirai commented Feb 26, 2015

Because src and style are not boolean attributes and behave differently.

@tilleps
Copy link

@tilleps tilleps commented Jun 2, 2015

I agree with @timoxley

+1 on keeping riot.tag()

@tipiirai how about riot-is-checked / riot-is-required / riot-is-* instead of ? or __

@tipiirai
Copy link
Contributor

@tipiirai tipiirai commented Jun 3, 2015

I prefer ? myself. The -is- notation sounds like an overkill for all the 45+ boolean attributes.

@tilleps
Copy link

@tilleps tilleps commented Jun 3, 2015

Not sure why it's considered overkill, as it would be (relatively) consistent with riot-src / riot-style / riot-tag. It just feels more intuitive/consistent to me.

While I understand that markup validation doesn't really matter these days, but I personally would prefer it to be data-is-* / configurable, as it is the only thing is keeping my riotjs markup from being validated.

Also if I'm not mistaken, starting an attribute name with ? is invalid and creates invalid xml/markup.

I understand that riotjs trying to minimize size, but I feel that having this configurable would be a great benefit / portability.

@tipiirai
Copy link
Contributor

@tipiirai tipiirai commented Jun 4, 2015

Actually – you are right. I played with various options on my text editor and riot-is-* or data-is-* feels best. I think this is the way to go.

Thanks!

@aMarCruz
Copy link
Contributor

@aMarCruz aMarCruz commented Jun 4, 2015

I think having configurable attributes adds too many overhead to the analyzer. The code becomes slow and complicated. e.g. see the brackets() function that lets you change the delimiters.

@tipiirai
Copy link
Contributor

@tipiirai tipiirai commented Jun 5, 2015

There will be no configurable attributes. We'll choose between riot-is- prefix or data-is- prefix and the compiler just checks for the existence of such attribute.

@web3d0
Copy link

@web3d0 web3d0 commented Nov 4, 2015

Bumping to look at #853 because Riot is so close to working with SVG as it is now, I don't think these issues are related. All I have to do is mount manually at console and it works with no errors. So why an error on load? This is not a prefix or syntax issue.

@aMarCruz
Copy link
Contributor

@aMarCruz aMarCruz commented Nov 24, 2015

Regarding boolean attributes, I corrected the list and today we have 29:
https://github.com/riot/compiler/blob/dev/doc/attributes.md.

@tipiirai , expressions in boolean attributes are working fine w/o the __ prefix for IE9 in Windows and Safari 5.1 in Mac Lion/Leopard (the problematic browsers). riot still needs to know when is a boolean to remove it on falsy values, but I think this can be done in the update function with low cost.

@tipiirai
Copy link
Contributor

@tipiirai tipiirai commented Nov 25, 2015

Happy to remove the __ prefix. And indeed, those were there for IE8 only.

@GianlucaGuarini
Copy link
Member

@GianlucaGuarini GianlucaGuarini commented Nov 25, 2015

@tipiirai @aMarCruz I agree but we must be sure that the next riot-tmpl has a method isBoolean to lookup these properties

@tipiirai
Copy link
Contributor

@tipiirai tipiirai commented Nov 25, 2015

Yeah. I hope there is a way to check that without the explicit list of these attributes and it works on IE9 too.

@GianlucaGuarini
Copy link
Member

@GianlucaGuarini GianlucaGuarini commented Nov 25, 2015

@tipiirai it's pretty simple I guess return ~booleanAttributes.indexOf(attribute) am I right @aMarCruz ?

@aMarCruz
Copy link
Contributor

@aMarCruz aMarCruz commented Nov 25, 2015

@GianlucaGuarini yes. Integrating #1297, this can be something like:

// at this point the attribute was removed
if (value && typeof value !== T_OBJECT)
  setAttr(dom, attrName, ~BOOL_ATTRS.indexOf(attrName) ? attrName : value)

..maybe for non-boolean attributes we need check for zero.

@aMarCruz aMarCruz self-assigned this Dec 2, 2015
@GianlucaGuarini GianlucaGuarini added this to the 3.0.0 milestone Feb 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.