Infinite loop when using :value and @input

[KeironLowe]

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

[x] Bug report => search github for a similar issue or PR before submitting
[ ] Feature request
[ ] Other, please describe

Tell about your platform

• flatPickr version : 8.1.5
• Vue.js version : 2.6.11
• Browser name and version : Chrome 83.0.4103.116

Current behavior
An infinite loop occurs and crashes the browser when selecting a date when using :value and @input instead of v-model. I use flatpickr inside another component hence we can't use v-model.

Expected behavior
The data is updated without a loop.

Minimal reproduction of the problem with instructions

Component Usage

<template>
    <date-picker :value="test" @input="test = $event" />
</template>
<script>
export default {
    data() {
        return {
            test: new Date()
        }
    }
}
</script>

DatePicker Component

<template>
    <flat-pickr :value="value" @on-change="$emit('input', $event[0])" />
</template>

<script>
import flatPickr from 'vue-flatpickr-component';
import 'flatpickr/dist/flatpickr.css';

export default {
    components: {
        flatPickr
    },
    props: {
        value: {
            required: true
        },
    },
}
</script>

[ankurk91]

You need to learn vue.js concepts first.
The example you have shared is the wrong implementation.

Please refer to package read me and use it accordingly

[KeironLowe]

@ankurk91 I'm sorry could you elaborate a little bit on what part is wrong? I read through the documentation prior to opening an issue. The only thing I can see is that I'm not using v-model but I can't use that as in reality the value is coming from Vuex.

[ankurk91]

You are mutating the data property via event, and binding the same property.

The Flat picker component is watching the value prop and emit input event, which might be causing the infinite loop.

What sre you trying to achieve, why can't you use the component directly.

Why do need another wrapper component?

[KeironLowe]

Isn't that the standard way v-model works on components though? It implements the same :value and @input that I've used?

I'm using a wrapper component because I have more code in reality, I only provided a minimal example as per the issue template. My component automatically handles adding a label element and provides a preconfigured setup for flatpickr.

[ankurk91]

@karllinden
Take this example
https://codesandbox.io/s/vue-flatpicker-7t62o

Hope this helps.

[KeironLowe]

Thanks, it actually looks like the issue only occurs when emitting a date object. If I emit a string then it works fine but if I convert it to a date object first then it loops. I would much prefer to emit a date object, is that possible?

I've updated your example with date objects to show the crash.

[ankurk91]

#50

#95

This has been discussed many times, it is not possible for now.

[renatodeleao]

Hi @ankurk91, thanks for you work on the wrapper first and foremost 🙏

I've ran into this today as well. It's a pretty common use case to wrap a component in a wrapper: either to provide custom markup and styles or extra logic. Composition and "controlled components" is an encouraged pattern in vue community.

Anyways, as @KeironLowe, In my case I just wanted to keep the model provided from consumer component (parent/form) to always keep the same value type: If i've provided an array [dateStr] as v-model, to keep things tidy, so I also want an array in return, reasons:

1. Developer experience, consistent input/output is expected
2. Avoid having to parse string values in consumer components like forms, before sending to server.
3. Useful if binding directly to vuex store values,
4. super useful if using range mode, since we input [startDateStr, endDateStr] and by default we receive a startDateStr to EndDateStr which relates to reason 2 (another usecase of mine)

Since flatpickr always returns as string as you've mentioned, i've naturally tried to do a wrapper that fitted my needs:

<template>
    <div class="my-extra-stuff">
      <flat-pickr
        :value="value"
        @input="onInput"
      />
  </div>
</template>

<script>
import flatPickr from 'vue-flatpickr-component'

export default {
  name: 'WrappedFlatPickr',
  props: ['value'],
  componets: {
    'flat-pickr': flatPickrComponent
  },
  data: () => ({
    originalInputType: null
  }),
  created() {
    // for testing purposes only, please don't do this
    this.originalInputType = Array.isArray(this.value) ? 'array' : 'string'
  },
  methods: {
    onInput(dateStr) {
      const parsedValue = this.originalInputType === 'array'
        ? dateStr.split()
        : dateStr
     
      console.log('wrapped:inputEvt')
      this.$emit('input', parsedValue)
    }
  }
}
<script>

<!-- on parent, and yes this is dummy example -->
<wrapped-flat-pickr v-model="someModelArrayWithDate" />

This is how v-model is supposed to work according to all official docs as @KeironLowe also mentioned.

The problem

I don't know flatpickr internals as well as you that's for sure, so i'm sure there's a reason behind it, but the culprit is the value watcher setup.

vue-flatpickr-component/src/component.js

Lines 179 to 185 in 8a5e434

	value(newValue) {
	// Prevent updates if v-model value is same as input's current value
	if (newValue === this.$el.value) return;
	// Make sure we have a flatpickr instance
	this.fp &&
	// Notify flatpickr instance that there is a change in value
	this.fp.setDate(newValue, true);

When you're notifying flatpickr instance with setDate(new, true), by passing the triggerChangeEvents flag as true that creates the loop, because onInput was already triggered by the fact that value has changed as far as i know, if flatpicker trigger it, triggers another input event, triggers another watcher, which is never stopped because you're doing a strict comparison of newValue with prevVal at the watcher handler.

watch: {
    value(newValue) {
      // will never stop if `newValue` is an `Array` and this.$el.value is a `string` (you won't notice at the beginning because this watcher is only called after first update)
      if (newValue === this.$el.value) return;
}

watch: {
    value(newValue, prevValue) {
      // will also not work because objects are not strict equals
      if (newValue === prevValue) return;
}

Proposed solutions:

1. perform the correct equality matcher, by coercing input values to string

watch: {
    value(newValue, prevValue) {
      // will prevent the loop.
      if (JSON.stringify(newValue) === JSON.stringify(prevValue)) return;
}

2. Do set triggerChange flag to false (from my testing events are still triggered anyways, but not 100% sure, and i'm not aware of side-effects of this)

   value(newValue, prevValue) {
      // Prevent updates if v-model value is same as input's current value
      if (newValue === this.$el.value) return;
      // Make sure we have a flatpickr instance
      this.fp &&
      // Notify flatpickr instance that there is a change in value

      // seeting this to true (emit events) will cause infinite loop on parent
      // components wrapping and manually binding events.
      this.fp.setDate(newValue, false);
    },
  },

Reproduction example

I've cloned your repo with some examples to help putting all this into context.

• https://github.com/renatodeleao/vue-flatpickr-component/tree/repro/infinite-loop (just set the flag to true in watcher to cause the loop). Also do as per solution#1 to stop it. Added some wrapped components to App.vue
• Fix infinite loop with controlled components haoxins/react-flatpickr#14 (also apparently react wrapper has suffered from the same thing).

Sry for the long post, i've tried my best to be clear on my explanation, but as a non-native english speaker, that can be exceptionally hard to do in short way. ✌️

[ankurk91]

@renatodeleao

Thanks for the write up, and possible solution.

I have gone through this issue initially when i was creating this component.
Thats why this component keep v-model and DOM (input value) same type string, and keep them is sync.
I found string is easy to watch and compare.

I am happy to implement and go with your approach. But i have some questions that needs clarification.

• How easy it is to watch a Date object or Array of Date Objects, Does Vue.js be able to detect such change in value prop?
• How easy it is to compare Date object or Array of Date Objects for equality.
• We need to rely on on-change event rather than input event to get Array of Objects from flatpickr instance. Need to check more.

I will invest some time on weekends to implement your solution.

[renatodeleao]

@ankurk91 thanks for looking at it, i'll do my best to try to clarify your points in order.

1. Date objects or arrays suffer from the same reactivity caveats as all objects in vue. But IMO that's the responsibility of the consumer, not vue-flatpickr: your wrapper should be kept aligned with the underlying library (flatpickr), so passing any supported input type should always output a string by default as per flatpickr.js. If people want, as I did, to do something different than default behaviour, like keeping same i/o format on v-model (let's say an array), they need to make a wrapper and ensure that the value they emit at their custom @input or @on-change implementation can be tracked down by vue, which trigger the watcher, but which the "new" diff logic can prevent the infinite-looping. Of course you can also bake that logic into the component and then provide a prop like same-io for it, but that will complicate your code a lot and this wrapper will not be a 1-1 match with flatpickr.js, so I don't think that's the way to go.
   -Reactive Date object SO
2. It's not easy at all to perform deep comparison. The good news is that all flatpickr (and therefore vue-flatpickr) supported input types can be easily coerced to strings with the JSON.stringify referred on my "solution#1", so the diff should be pretty easy:

   // supported v-model input types: null, dateString, Array of dateStrings, Date object, Array of Date Objects, 
   JSON.stringify(null) // "null"
   JSON.stringify("2020-07-01") // ""2020-07-01""
   JSON.stringify(["2020-07-01", "2020-07-02"]) // "["2020-07-01", "2020-07-02"]"
   JSON.stringify(new Date()) // ""2020-07-01T14:16:44.267Z""
   JSON.stringify([new Date(), new Date()]) // "["2020-07-01T14:22:15.055Z","2020-07-01T14:22:18.721Z"]"

3. I understand that @on-change includes selectedDates as first argument contrary to @input, but if the consumer wants to use one or another that's their responsibility. Your wrapper just need to make sure that the flatpickr instance is fed with the latest value and responds accordingly (like not looping if the value is the same :p). (at least that's what i expect from vue wrapper component).

I really hope that this was helpful, if you want I can review your PR when ready or make one myself.

Cheers ☮️

[aqualaguna]

hit this issue as well is there any work around? for quick fix or there another version release to fix this issue?

[aqualaguna]

<template>
  <flat-pickr
    v-model="clone_value"
    @input="onInput"
    :config="config"
  />
</template>
<script>
import flatPickr from 'vue-flatpickr-component';
import 'flatpickr/dist/flatpickr.css';

export default {
  name: 'FlatPickrWrapper',
  components: {
    flatPickr
  },
  props: {
    value: {
      required: true,
      default: ''
    },
    config: {
      type: Object,
      default: () => ({})
    }
  },
  data() {
    return {
      clone_value: new Date(this.value),
    }
  },
  watch: {
    // value(val) {
    //   this.clone_value = val instanceof Date ? val.toString() : val;
    // }
  },
  methods: {
    onInput(value) {
      this.$emit('input', new Date(value))
    }
  }
}
</script>

this is my quick fix hahaha :) just clone the value and done. but still you need fix this issue

<flatpickr-wrapper
        class="flex-initial ml-2 p-2 mt-2"
        v-model="range.from"
        :config="pickerConfig"
      />

voila!!
flatpickr with emit date object not string.

[ankurk91]

@renatodeleao
Take a look at this branch
https://github.com/ankurk91/vue-flatpickr-component/tree/issue-190

Checkout to issue-190 branch and npm run dev to play with examples

Notice that

• v-model is always Array now, even if there is one date (we can fix this, based on flatpickr mode)
• The DOM (Input) would still have string value, so normal form post will send string to server, while AJAX form post will send Array of serialised Date Objects
• No need for additional wrapper component, this component can itself behave the way you want
• No need to set false to setDate() method
• We can still rely on input event by utilising flatpickr's internal property selectedDates
• End consumer need to ensure that value prop can detect changes to Array by using Vue.$set()
• JSON.stringify converts date object to UTC format, server need to parse and convert it accordingly.

[renatodeleao]

Hey @ankurk91 sry for the delay in the response.
I don't see anything wrong with your approach, but can't this be considered a breaking change? You're changing the expected output for anyone using anything up to v8.1.5 (which till now was always a string). More detailed:

• "v-model is always Array now (...)" — from what i've seen other pickers do, when single mode is on a single date object is emitted. But again if flatpickr.js doesn't do that with it's own on-change method, maybe this wrapper shouldn't as well. Up to you to decide.
• "The DOM (Input) would still have string value (...)" — yes that is the responsibility of the flatpicker.js which updates the value and it should continue to be string (from what i can see, you don't bind the value prop to the input, so nothing changed there)
• "No need for additional wrapper (...)" — well for me it doesn't, but tomorrow some will show up saying that they need this to emit an object with start and end date keys, so you know the drill, wrappers will always happen, you just need to make sure your component handles different supported inputs nicely (no looping). Of course you can always advise to use a specific format for v-model and use a computed property on the parent if they want it converted to their fancy format.
• "No need to set false to setDate() method (...)" — yup we can still notify fp instance, thanks to new watcher logic. EDIT: but by setting to true, well see the onchange event trigger twice (one manually due to the value change, and another emitted by flatpickr )
• "End consumer need to ensure that value prop can detect changes to Array by using Vue.$set()" — that or creating a new array entirely, but that's up to them.
• "JSON.stringify converts date object to UTC format, sever need to parse and convert it accordingly" — didn't get this one, we just stringify values in the watcher for comparison, so this does not affect emitted values. The value that fp "injects" into the input is still the string value with the dateFormat as before.

A question though, relative to the watcher:

value(newValue) {
     if (JSON.stringify(newValue) === JSON.stringify(this.fp.selectedDates)) return;
}

Any particular reason why you use this.fp.selectedDates instead of previousValue that can be provided by adding a second argument to the watcher handler?

Cheers ☮️