Regression in DateInput

[ggregoire]

Hi guys!

I got a regression in updating from 1.1.0 to 1.2.0 (latest).

The bug comes from this PR.

The component crashes during its instantiation because of this change.

Some screenshots:

It worked before to pass a string to the component because of the moment(date).

Now it doesn't work anymore because of the moment([date.getFullYear(), ...]).

To be honest, your doc says that value should be a Date and not a string, so...

We could indicate in the changelog the potential breaking change and a possible solution, e.g.

replacing value={value} by value={new Date(value)}

Or maybe a workaround in the code to keep the previous behaviour?

[cmslewis]

Thanks for filing, @ggregoire! This is helpful context, but I'm going to close this as a duplicate so that we can track the issue in a single place in #280. Feel free to chime in over there.

[adidahiya]

Actually the correct duplicate is #322. Fixed for today's upcoming release.

[ggregoire]

Thanks for filing, @ggregoire! This is helpful context, but I'm going to close this

Fixed for today's upcoming release.

Guys, did you read my bug description beyond the first 3 lines?

How this fix could fix this?

Could you change this

if (null == date) {
    return moment(null);
}

to

if (null == date || typeof date === "string") {
    return moment(date);
}

for today's upcoming release?

Thanks.

[ggregoire]

Some tests to avoid future regressions:

it("renders without crashing", () => {
  const DATE_NULL = null;
  ReactDOM.render(<DateInput value={DATE_NULL} />, document.createElement("div"))

  const DATE_STRING = "1988-08-07 11:01:12";
  ReactDOM.render(<DateInput value={DATE_STRING} />, document.createElement("div"))
})

[adidahiya]

Hey, you're right @ggregoire, sorry I overlooked that. Opened a new PR #342 which we will try to merge for today's upcoming release.

[ggregoire]

@adidahiya @cmslewis @giladgray Thanks for looking into it! :)

[cmslewis]

Whoops, sorry about that @ggregoire. Thanks for staying vigilant.