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

Minor bug with Boolean props casting: undefined => false #4792

Closed
frandiox opened this issue Jan 26, 2017 · 28 comments
Closed

Minor bug with Boolean props casting: undefined => false #4792

frandiox opened this issue Jan 26, 2017 · 28 comments
Labels

Comments

@frandiox
Copy link

Vue.js version

2.1.10

Reproduction Link

https://jsfiddle.net/nymhjosc/

Steps to reproduce

Simply omit a Boolean prop and the value will be casted to false.

What is Expected?

I'd expect to have an undefined property just to know that the user didn't provide it (since it is not a required prop). Basically, I would expect both myProp and missingProp in the previous example to be the same.

What is actually happening?

When the prop is not provided its content is casted to false. Also, some other values like '' are casted to true (somewhat related to #4538) but I guess this is intended. I'm using default: undefined for now.

@znck znck added the bug label Jan 26, 2017
@LinusBorg LinusBorg added the 2.1 label Jan 26, 2017
@posva
Copy link
Member

posva commented Jan 26, 2017

Since you give the prop a Boolean type, it's normal to have a false value for the missing prop. What bothers me more is the undefined value passed to a Boolean prop not being cast to a boolean or getting an error. The same happens with null

@znck
Copy link
Member

znck commented Jan 26, 2017

@posva Sometimes undefined is required.

I use it for modals so parent can control the state of modal with prop a value and if prop is undefined then parent can trigger by open event.

@posva
Copy link
Member

posva commented Jan 26, 2017

@znck Yeah, I cannot remind where I read something similar, but being able to explicitly pass undefined to any type of prop is probably a feature

@znck
Copy link
Member

znck commented Jan 26, 2017

being able to explicitly pass undefined to any type of prop is probably a feature

Passing undefined works, the bug is when a prop is not provided by user component then naturally it is expected to be undefined.

@posva
Copy link
Member

posva commented Jan 26, 2017

I don't think so, because we're asking for a Boolean type, so a missing prop is false, which is more intuitive.
To keep undefined when the prop is missing we can just use a null type

@simplesmiler
Copy link
Member

simplesmiler commented Jan 26, 2017

@znck yeah, but what is the purpose of default then, when would it be called?


EDIT: Nevermind, the issue is not about default.

@frandiox
Copy link
Author

@posva Honestly, the same way a missing String prop does not become '', I would expect a missing Boolean to be undefined and not false. But I guess it's a matter of opinion.

And the other thing I pointed out

some other values like '' are casted to true

is also kind of weird to me. In JavaScript an empty string is a falsy value.

@yyx990803
Copy link
Member

The boolean casting follows the same rule of a boolean attribute: presence of any value are casted to true, absence means false.

Personally I think it's a bad idea to differentiate a "3rd state" for boolean props.

@realcarbonneau
Copy link

realcarbonneau commented Jan 20, 2018

Although I highly respect all of your opinions and knowledge, I would like to present a different view.

As a developer, I may want to know if a Boolean prop was defined or not.
Here are two examples:

  1. As a component developer for external use, I may want to build some logic to determine if the property was defined or not for various internal checks and robustness. And, it is not realistic to impose the use of :my-bool="undefined", especially when trying to make a component robust and flexible. Fortunately, there is a workaround that does not impose external changes, that is to explicity set the default to undefined: boolProp: {type: Boolean, default: undefined}

  2. A multi-value prop that includes a boolean value may be desired. For example happyIcon: [Boolean, String]. This simplifies the usage of the component (which assumes happy by default) with the following example scenarios for the prop:
    a) false: no icon displayed
    b) undefined, defined or true: display the default icon
    c) string: custom icon name

However, since vue forces undefined to false, I cannot manage case b since I cannot observe the undefined state and would be forced to add a second prop, thus requiring two props (Boolean and String) to do something that could be done with one. Additionally, I cannot just ask the user to pass a string "false" or "true" in the string since this could be an icon name.

Since undefined is fasly, there is no negative impact in keeping it, however, there is a loss of functionality by removing it.

As an additional irritant, if I define a Boolean and String prop with undefined as a default and the prop is defined, the component receive an empty string instead of the expected true value. (Maybe I should submit this as a seperate issue?)

    bsu: { 
      type: [Boolean, String], 
      default: undefined
    },

See the following jsfiddle for the examples: https://jsfiddle.net/realcarbonneau/daxs9722/

@vincerubinetti
Copy link

The main use case I can see for having a third state for a boolean is HTML attributes:

<div data-blah="true">
<div data-blah="false">
<div>

@bryanmylee
Copy link

I'm just getting started with Vue and this behaviour was definitely a surprise to me.

I'm using the composition API with the defineProps<{ open?: boolean | undefined }>() macro, and I expected open to be undefined if omitted.

It's weird that props.open is typed as boolean instead of boolean | undefined, but I guess I'll just have to get used to that footgun.

@mrweiner
Copy link

Chiming in years later to say that this confounded me as well. Since we are using https://github.com/gcanti/fp-ts, I opted to make the prop an Option type, which essentially provides the desired functionality.

const props = defineProps({
  forceIsVisible: {
    type: Object as () => O.Option<boolean>,
    required: false,
    default: O.none,
  },
});

Doubt it's worth installing this package just for this but it does the trick.

@klapAtGitHub
Copy link

klapAtGitHub commented Jun 8, 2023

I'd vote for undefined as default value for 2 reasons:)

  • other types (number, string and object) default to undefined. So the boolean should not be an exception.
  • if a prop can be marked as optional (not required), it sounds for me like a nullable prop. And I think it should be possible to know if it has been omitted, instead of changing it to any valid value. This could disguise errors, I think. But I understand the other arguments as well.

However, the workaround from frandiox works well (explicitely specifying an undefined or null as default value).

aBooleanProp: {
    type: Boolean,
    default: undefined // to be consistent with other types
}

Is that the recommended and save way to handle optional boolean props?
Or should I use a wrapper object for optional types as mrwiner suggested?

@matthew-dean
Copy link

matthew-dean commented Jun 14, 2023

This is still a weird part of Vue's API, and violates the Law of Least Surprise.

Take this statement by @yyx990803:

The boolean casting follows the same rule of a boolean attribute: presence of any value are casted to true, absence means false.

Personally I think it's a bad idea to differentiate a "3rd state" for boolean props.

All attributes in HTML are this way, not just booleans. Open up Chrome, inspect an element, and select Properties. You will see no undefined values! If it's a string, it will default to "". If it's a number, it will default to 0 or -1 (depending on what they represent). If it's a boolean, it's true or false.

It's absolutely counter-intuitive to treat booleans differently. Either all Vue properties should have default values, by default, or none should.

More importantly, though, from a component perspective, is this statement by @realcarbonneau:

As a developer, I may want to know if a Boolean prop was defined or not

Because these are developer-to-developer JavaScript and TypeScript interfaces, while it may seem a "bad idea to differentiate a 3rd state for boolean props" to some, this is a core staple of the JavaScript / TypeScript language itself. i.e. developers want to specify a behavior for when a property is not set at all. We can call it a "3rd state", but it's a 3rd state in the way that omitting an object property or omitting a function's argument is a 3rd state for that variable / value. That is, all types in JavaScript / TypeScript have third states (except in the latter case when null or undefined is explicitly omitted). So there is developer value in defaulting to undefined. In fact, we know there is, because every other type defaults to undefined.

In fact, this gets even more off-kilter when TypeScript support was increased for Vue, and you have interfaces like this:

interface Props {
  name?: string
  isCool?: boolean
}

In TypeScript, both of these properties are defined as optional, which means automatically that their respective values are string | undefined and boolean | undefined. Despite the syntax being exactly the same, Vue's TypeScript implementation handles the interface inconsistently, defining name as string | undefined (instead of string) and defines isCool as boolean.

So, there's little rational reason why booleans not only behave inconsistently from an HTML perspective, and not only behave inconsistently from a Vue perspective, but also behave inconsistently from a JavaScript and TypeScript perspective. It really is a complete oddity in this legacy behavior.

@IcyFoxe
Copy link

IcyFoxe commented Jun 15, 2023

I just had this same issue today.

If I define a prop as someProp?: string and it will be undefined, when no attribute is passed to the prop,
why doesn't someProp?: boolean behave the same way??

Worst part is that TypeScript understands it exactly as that - boolean or undefined, but in reality it's always boolean.

@matthew-dean
Copy link

matthew-dean commented Jul 17, 2023

If I define a prop as someProp?: string and it will be undefined, when no attribute is passed to the prop,
why doesn't someProp?: boolean behave the same way??

It's a part of the Vue API that makes zero rational sense. In JavaScript (and TypeScript), Booleans are not a different kind of primitive from other primitives. And, yes, it violates the explicit definition of the type, so it's 100% a TypeScript bug, even if Vue maintainers want to insist it's not a Vue bug.

@zernonia
Copy link

zernonia commented Oct 7, 2023

Yup this issue is quite prominent when building radix-vue, where we expose the Props & Emits type for dev to build wrapper component around it. It makes it super hard to build wrapper components, as I cant expect the dev to always define the props with withDefaults and manually set those boolean props to undefined.

Also, by casting the boolean to false really surprised me as I'm expecting the type to be undefined (explained by above comment nicely). I gotta agree with @matthew-dean in terms of violating Law of Least Surprise.

To workaround this issue, I've created a useForwardProps, showcasing on shadcn-vue and unovis/vue.

This is indeed a pain right now...

@dondevi
Copy link

dondevi commented Dec 1, 2023

Here is a hack to solve the problem:

function isPlainObject(value) {
  return value?.constructor === Object;
}

app.mixin({
  beforeCreate() {
    /**
     * Fix default value to `undefined` of Boolean props when inheriting
     */
    Object.keys(this.$props).forEach((key) => {
      const prop = this.$props[key];
      const type = this.$.type.props[key];
      if (isPlainObject(type) && prop === false && type.default === undefined) {
        /**
         * @hack `props` is readonly, use `props.__v_raw` to get a mutable `Proxy`
         */
        this.$props.__v_raw[key] = undefined;
      }
    });
  },
});

💡 A new version see below.

@sadeghbarati
Copy link

Make Runtime Props Validation Optional Please 🙏

Instead show "Vite Error Overlay" on screen

With Casting Props in runtime we would never have Complex Types or Entire Props Type Object support for defineProps/emits macro

@matthew-dean
Copy link

matthew-dean commented Jan 23, 2024

@dondevi Thank you SO MUCH for fixing this!

EDIT: Your code comment says "Fix default value to undefined of Boolean props when inheriting"... does this not set boolean props to undefined in all cases?

@dondevi
Copy link

dondevi commented Jan 26, 2024

@dondevi Thank you SO MUCH for fixing this!

EDIT: Your code comment says "Fix default value to undefined of Boolean props when inheriting"... does this not set boolean props to undefined in all cases?

Because of the beforeCreate() hook, for all component instances, as long as a prop value is false, and it's type object missing default field, it means that value is not from withDefaults(), so we could reset it to undefined.

@matthew-dean
Copy link

matthew-dean commented Jan 27, 2024

@dondevi I changed it to this, because props is not always a guaranteed object under type:

      for (const [key, prop] of Object.entries(this.$props)) {
        const type = this.$.type?.props?.[key]
        if (type && prop === false && !('default' in type)) {
          /**
           * @hack `props` is readonly, use `props.__v_raw` to get a mutable `Proxy`
           */
          this.$props.__v_raw[key] = undefined
        }
      }

There might also be an opportunity to optimize for re-mounting, because the props keys will not change between instances, so you technically only have to iterate boolean keys, but I couldn't figure out how to get the original component

@matthew-dean
Copy link

matthew-dean commented Jan 27, 2024

@dondevi Wait a minute... I think your logic is flawed....

If I have a component like this:

<script setup lang="ts">
defineProps<{
  bool?: boolean
}>()
</script>

... then it has no default value set. And if I then explicitly set bool to false, then will your code not set it to undefined? 🤔 You're assuming that any false value is not explicit, which may not be the case. I don't think this can solve the issue.

@matthew-dean
Copy link

@dondevi I think this comment actually ends up being the better workaround.

Then your component can be like (note capital Boolean):

<script setup lang="ts">
defineProps<{
  bool?: Boolean
}>()
</script>

As long as Vue never breaks this behavior, then I think that's a suitable workaround that should be more publicized.

@dondevi
Copy link

dondevi commented Jan 29, 2024

@dondevi Wait a minute... I think your logic is flawed....

If I have a component like this:

<script setup lang="ts">
defineProps<{
  bool?: boolean
}>()
</script>

... then it has no default value set. And if I then explicitly set bool to false, then will your code not set it to undefined? 🤔 You're assuming that any false value is not explicit, which may not be the case. I don't think this can solve the issue.

@matthew-dean Thanks for pointing it out, it won't cover all scenarios as it's a hack to attract good ideas.

The logic has been updated with isPlainObject().

@dondevi
Copy link

dondevi commented Apr 18, 2024

@matthew-dean A better solution:

/**
 * @hack Prevent vue's behavior of casting boolean default value to false
 * @since vue 3.3+
 * 
 * @see {@link vuejs/core/packages/runtime-core/src/componentProps.ts:484#resolvePropValue} - boolean casting
 */
app.mixin({
  beforeCreate(this: ComponentPublicInstance) {
    const { props, type, vnode } = this.$;
    const types = Object.entries(type.props || {}) as Array<[string, any]>;

    for (const [key, prop] of types) {
      const raw = vnode.props?.[key];
      const value = props[key];
      if (
        ([] as unknown[]).concat(prop.type).includes(Boolean) // Has boolean type
        && prop.required !== true && !('default' in prop) // Optional without default
        && raw === undefined && value === false // Casted to false by vue
      ) {
        /**
         * Reset boolean props value to undefined
         * @hack `props` is readonly, use `props.__v_raw` to get a mutable `Proxy`
         */
        (props as any).__v_raw[key] = undefined;
      }
    }
  },
});

@tmdkamins
Copy link

This definitely violates principle of least surprise when all (?) other types default to undefined when not specified. But changing this behavior would be a breaking change and is not going to happen in Vue 3. See comment from core dev: vuejs/core#9882 (comment)

A couple workarounds:

  1. Use Boolean (big B) instead of boolean.
  2. Provide a default value (withDefaults) of undefined.

Either of these seem to elicit the expected behavior.

@dondevi
Copy link

dondevi commented Apr 23, 2024

I have to do it for this kind of scene:

Though props in QInput have default values, but they can't extend by ts

<template>
  <q-input
    v-bind="props"
    ...
  />
</template>

<script lang="ts" setup>
import { QInput, QInputProps } from 'quasar';

interface Props extends QInputProps {
  ...
}

const props = defineProps<Props>();
</script>

The paradox is that before vue 3.3-, I didn’t need to write v-bind="props" and could achieve perfect extend.
This may be a bug fixed in vue 3.3+. So when I upgraded, I was in big trouble.

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