Don't pass undefined arguments through to Flatpickr config

[gitKrystan]

The problem

We would like to wrap <EmberFlatpickr> in a component to DRY up some of the config options that we pass to every instance. Because Ember doesn't have "splargs" (a la splattributes), this means we end up having to pass through every possible Ember Flatpickr arg like so:

<EmberFlatpickr
  class="w-full block border-2 p-3 mb-2"
  placeholder="Choose a date"
  ...attributes
  @allowInput={{or-default @allowInput true}}
  @altFormat={{@altFormat}}
  @altInput={{or-default @altInput true}}
  @altInputClass={{@altInputClass}}
  @clickOpens={{@clickOpens}}
  @date={{@date}}
  @dateFormat={{@dateFormat}}
  @defaultDate={{@defaultDate}}
  @defaultHour={{@defaultHour}}
  @defaultMinute={{@defaultMinute}}
  @disable={{@disable}}
  @disabled={{@disabled}}
  @disableMobile={{@disableMobile}}
  @enable={{@enable}}
  @enableSeconds={{@enableSeconds}}
  @enableTime={{@enableTime}}
  @hourIncrement={{@hourIncrement}}
  @inline={{@inline}}
  @locale={{@locale}}
  @maxDate={{@maxDate}}
  @minDate={{@minDate}}
  @minuteIncrement={{@minuteIncrement}}
  @mode={{@mode}}
  @monthSelectorType={{or-default @monthSelectorType 'static'}}
  @nextArrow={{@nextArrow}}
  @noCalendar={{@noCalendar}}
  @onChange={{@onChange}}
  @onClose={{@onClose}}
  @onOpen={{@onOpen}}
  @onReady={{@onReady}}
  @parseDate={{or-default @parseDate this.parseDate}}
  @prevArrow={{@prevArrow}}
  @shorthandCurrentMonth={{@shorthandCurrentMonth}}
  @static={{@static}}
  @time_24hr={{@time_24hr}}
  @weekNumbers={{@weekNumbers}}
/>

Even when an argument is undefined, Ember still passes it through. Thus, Ember Flatpickr passes undefined into the flatpickr function. Eventually, this makes its way to this line in Flatpickr:

https://github.com/flatpickr/flatpickr/blob/07cf1b1ba5ec71da511c295f622d60eed3bf3eb7/src/index.ts#L2039

Object.assign will overwrite all named properties from the target object even if the value is undefined, so this overwrites the default config with invalid undefined config. Eventually, this results in type errors while trying to initialize the Flatpickr object. For example, this is the error thrown in my "it will not override Flatpickr defaults with undefined" test if you remove the code that fixes the issue in _setFlatpickrOptions:

            TypeError: Cannot set property 'disabled' of undefined
                at EmberFlatpickr._setDisabled (http://localhost:7357/assets/vendor.js:122353:26)
                at EmberFlatpickr._setFlatpickrOptions (http://localhost:7357/assets/vendor.js:122333:12)
                at invoke (http://localhost:7357/assets/vendor.js:59886:16)
                at Queue.flush (http://localhost:7357/assets/vendor.js:59775:13)
                at DeferredActionQueues.flush (http://localhost:7357/assets/vendor.js:59972:21)
                at Backburner._end (http://localhost:7357/assets/vendor.js:60506:34)
                at Backburner._boundAutorunEnd (http://localhost:7357/assets/vendor.js:60175:14)

My solution

Filter undefined values from the config object before passing it through to the flatpickr function. For the most part, undefined seems to be an invalid config option in Flatpickr (per https://flatpickr.js.org/options/), with null preferred for turning things "off." The one exception seems to be the enable property, which defaults to undefined (so not passing in undefined to override undefined is probably OK.)

Other notes

• There seems to be a failing test on master (at least when I run them locally). This PR does not fix that failing test.
• When I tried to yarn install, I got an error because one of the dependencies has deprecated Node 10, which is what's specified in the volta config for this project. To fix, I upgraded node for the project locally but did not commit this change.

[RobbieTheWagner]

Hi @gitKrystan thanks for the PR! I could be wrong, but I think ember-flatpickr is setup to not pass anything you did not pass to it, so this issue only arises for your case because you are explicitly passing things like @foo={{undefined}} correct? I would recommend handling this in your wrapper component and not passing in those undefined values into ember-flatpickr, but I am not necessarily opposed to adding these guards in the addon itself.

[chancancode]

@rwwagner90 it's basically an Ember/Glimmer limitation right now. When writing a wrapper component, there is no way to "optionally pass an argument", like you can't say <EmberFlatpicker {{if @foo @foo=@foo}} /> or <EmberFlatpicker @foo={{if @foo @foo (nevermind)}} /> or <EmberFlatpicker ...@arguments />or<EmberFlatpicker ...this.myArgs />`.

So when writing a wrapper component you have to bind all of the possible arguments, but that means anything that is not passed from the invoker will become @foo={{undefined}}. Usually it doesn't matter that you passed undefined, but in this case it does.

Don't love any of these, but those are the limitations we have to work with right now; having "splat args" would fix all of these.

[RobbieTheWagner]

@chancancode wouldn't @foo={{if @foo @foo null}} work for this case?

[gitKrystan]

@rwwagner90 @foo={{if @foo @foo null}} would have a similar issue of unintentionally overwriting Flatpickr defaults with null.

[RobbieTheWagner]

@rwwagner90 @foo={{if @foo @foo null}} would have a similar issue of unintentionally overwriting Flatpickr defaults with null.

Ahhh, thank you, I understand now.

[chancancode]

Yeah, it's an unfortunate reality at this moment.

Usually, undefined is more or less how you express "absence" in JavaScript.

For example:

• let foo; is the same thing as let foo = undefined;
• function f(arg = 'default') {}, f() and f(undefined) will give you the same result
• JSON.stringify({ foo: 'foo', bar: undefined }) is the same as JSON.stringify({ foo: 'foo' })

So if anything, @foo={{if @foo @foo undefined}} would be what you want here, but that is really just the same as @foo={{@foo}}. Generally, you can and should treat those "absence" and undefined interchangeably, and that mostly works and doesn't cause any issues.

Unfortunately in the specific case someone ultimately cares about the difference between undefined and "absent". Arguably it's Object.assign's "fault"? Since in the hbs there isn't any other way to express "absence" and there is no spreading of arguments, this is what we are stuck with for the time being.