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

🎷 confusing dom-if / falsey detection for initial values #2447

Closed
samccone opened this issue Sep 17, 2015 · 10 comments
Closed

🎷 confusing dom-if / falsey detection for initial values #2447

samccone opened this issue Sep 17, 2015 · 10 comments

Comments

@samccone
Copy link
Contributor

◼️ Given

<template is="dom-if" if="[[zap]]">
  <b>wow</b>
</template>
<template is="dom-if" if="[[!zap]]">
  <b>tank</b>
</template>
...
properties: {
   zap: String
},
attached: function() {
  this.setTimeout(function() {
    this.zap = 'doge';
  }.bind(this), 1000)
}
...

🌖 Expected

  1. tank should show
  2. then after 1 second wow should show (tank should not be showing now)

🌍 What happens IRL

tank never shows... and then wow shows (in 1 second)

✏️ Hypothesis

because zap is never "defined" polymer does not try and negative a value that has not been resolved

💻 🔨 Hack fix

properties: {
-   zap: String
+   zap: {
+     type: String,
+     value: ''
+   }
},

🍝 Ideal fix

In a dom-if... it should try and eval zap right away, because ideally I would like to be able to handle the "missing" case without having to define a falsey string val.

Thanks for the ⌚
Let me know if this is a terrible idea... feel free to close 💀 🙉

@tomalec
Copy link
Contributor

tomalec commented Sep 20, 2015

👍

@samccone
Copy link
Contributor Author

ping @tjsavage -- This is something that keeps being a headache across applications that I am building, any thoughts?

@samccone
Copy link
Contributor Author

samccone commented Nov 4, 2015

another ping on this issue 📟

I think the use-case of this can be expressed quite simply:

I want to hide a content area while a scope var is not defined, and instead show a loading indicator..

you would think we could do

dom-if="[[!foo]]" however since foo is not defined this expression is not evaluated, Instead I have to introduce a new tmp var / method in my logic that gets computed off of foo, and initially is set to false... thus increasing the amount of state that I have to hold on to just to deal with this evaluating flow.

I would love to hear how other people are dealing with this, because it is something I continually hit when building async applications where data flows into my "state" post render
.

@tjsavage
Copy link
Contributor

tjsavage commented Nov 5, 2015

I think this is quite reasonable, and probably fits into our roadmap goal of "Fix and make usage of undefined consistent throughout the library." The plan is to comb through all the places where undefined makes an impact and make sure they're all aligned, so this should certainly be looked at as part of that process.

@samccone
Copy link
Contributor Author

samccone commented Nov 5, 2015

some non breaking syntax ideas to enable this behavior... (evaluating foo even if it is undefined)

<template is="dom-if" if="[{[foo]}]">
<template is="dom-if" if="[[[foo]]]">
<template is="dom-if" if="{{{foo}}}">

@nazar-pc @JeremybellEU any thoughts in this area?

@TimvdLippe
Copy link
Contributor

The syntax might be troublesome for new developers whom would confuse the 3 { with the regular 2, which might introduce programming errors.

In the last hour I have tried to understand the annotation setup mechanics. I have found when things are initiated, but sadly lack the experience where all calls land (they jump between files). For me it seems most logical if all properties are initially undefined and the property would be evaluated right away. Since !undefined === true, this issue would be solved.

Evaluating right away might lead into problems of ordering of properties, as shown in #2674 .

@nazar-pc
Copy link
Contributor

nazar-pc commented Nov 5, 2015

I personally don't have problems with [[!zap]]. The reason why is simple, literally from the first day of switching to Polymer 1.x I've started to send PRs with different bugs/features and since I really need them I'm using patched version myself and maintaining/rebasing it against upstream constantly.

PR I want to mention here is #2205, it basically does what Hack fix section suggests but with nice syntax:

properties: {
   zap: ''
},

I'm not sure it solves initial problems, I also had difficulties with undefined in some places, but anyway this particular issue is easily avoided without any pain.
BTW, PR does not as nice as I'd like it to be, but it works, give it a try:)

@dfreedm
Copy link
Member

dfreedm commented Nov 6, 2015

It seems like a bug that the default value of a type is not used when defining it in Properties.
As @tjsavage said, this should tie into the cleanup of undefined in the data binding system.

@sasivarnan
Copy link

+1

Facing the same issue with Boolean type also.
http://plnkr.co/EgdYKE

@tjsavage tjsavage added the 1.x label Sep 8, 2016
@TimvdLippe TimvdLippe self-assigned this Nov 28, 2017
@TimvdLippe TimvdLippe added this to Planned in Embarcadero walk Nov 28, 2017
@TimvdLippe
Copy link
Contributor

We have discussed this use-case and decided that it is working as intended. As the data-binding system of Polymer relies on a push-model (e.g. when you set values, side effects like bindings run), a value that has yet to be set (undefined) causes no side effects to occur. Thus, if you want to "push" the value and update the binding, you should use a value other than undefined. Luckily this should be straightforward with any other falsy value, such as null.

We do agree that this notion is different than the falsiness of undefined in JavaScript, but this is a consequence of the push model used in data-bindings. Thus I am closing this issue as working as intended.

Embarcadero walk automation moved this from Planned to Closed Nov 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Embarcadero walk
  
Closed
Development

No branches or pull requests

8 participants