Datetimepicker: Wrong type declaration in the onChange event

Created on 9 Dec 2020  路  10Comments  路  Source: react-native-datetimepicker/datetimepicker

Bug report

Summary


The Event received as the argument for the onChange callback has an incorrect type declared. The type should be event.nativeEvent.timestamp but the actual type declared is evt.nativeEvent.timeStamp. Note the uppercase S in
timeStamp

This is not a problem since I can do what I need using the second argument on the onChange callback or even using event.nativeEvent.timestamp and disregarding the linter. Anyways, I thought maintainers would like to know this is happening. Hopefully it should be an easy fix for someone familiar with React Native types.

Reproducible sample code

This shows a linting error:

<DateTimePicker
  value={new Date()}
  mode='date'
  is24Hour={false}
  display='default'
  onChange={evt => {
    const timestamp = evt.nativeEvent.timestamp
    if (timestamp) {
      // handle the input change
    }
  }}
/>

Steps to reproduce

Using the code above the linter will complain and suggest to use evt.nativeEvent.timeStamp instead.

Describe what you expected to happen:
I expected timeStamp to be the property with the value, but it was timestamp. The result was that timestamp in the code above was always undefined

Hovering over event.nativeEvent shows the following type:

React.BaseSyntheticEvent<Event, EventTarget & Readonly<{ timestamp: number; }>, EventTarget>.nativeEvent: Event

I was hoping the Readonly<{timestamp: number;}> part should fix this, but I've been using typescript for just a few days and I don't really know what's going on there

Environment info

npx react-native info output:

System:
    OS: macOS 10.15.5
    CPU: (4) x64 Intel(R) Core(TM) i5-6360U CPU @ 2.00GHz
    Memory: 32.04 MB / 8.00 GB
    Shell: 5.7.1 - /bin/zsh
  Binaries:
    Node: 12.16.1 - /usr/local/bin/node
    Yarn: 1.3.2 - /usr/local/bin/yarn
    npm: 6.13.4 - /usr/local/bin/npm
    Watchman: 4.7.0 - /usr/local/bin/watchman
  Managers:
    CocoaPods: 1.10.0 - /usr/local/bin/pod
  SDKs:
    iOS SDK: Not Found
    Android SDK:
      API Levels: 23, 24, 26, 29
      Build Tools: 23.0.1, 26.0.1, 26.0.2, 28.0.3, 29.0.2
      Android NDK: Not Found
  IDEs:
    Android Studio: Not Found
    Xcode: /undefined - /usr/bin/xcodebuild
  Languages:
    Java: 1.8.0_141 - /usr/bin/javac
    Python: 2.7.16 - /usr/bin/python
  npmPackages:
    @react-native-community/cli: Not Found
    react: 16.13.1 => 16.13.1
    react-native: ~0.63.3 => 0.63.3
    react-native-macos: Not Found
  npmGlobalPackages:
    *react-native*: Not Found

datetimepicker version: ^3.0.8

iOS / Android version: Android 11

Most helpful comment

Interesting.

I've gone a bit more in depth looking those links, and then came back to my code and drilled down the types with cmd + click. I think I've found something.

I'm going to detail all the steps just to make sure I didn't jump to the wrong place by mistake, but that's going to make this a bit lengthy. I'll add a TL;DR version.

TL;DR

I think the Event declared here should be sending the Readonly type as the second generic, and probably unknown as the first. Something like this

export type Event = SyntheticEvent<
  unknown,
  Readonly<{
    timestamp: number;
  }>
>;

Doing that actually fixes the error the linter was giving me.

If, after reading the long version, you think that's a valid solution for this issue, I'll be happy to open a PR with the change.

Long version

I started from this definition [source]

declare const RNDateTimePicker: FC<
  IOSNativeProps | AndroidNativeProps | WindowsNativeProps
>;

Then I went to find onChange on IOSNativeProps | AndroidNativeProps | WindowsNativeProps. All of them extend BaseProps. Inside BaseProps there is DateOptions [source], DateOptions extends BaseOptions [source] and inside BaseOptions I found the onChange declaration [source].

onChange?: (event: Event, date?: Date) => void;

That Event type is what we're talking about, and it's declared at the top of the file [source]

export type Event = SyntheticEvent<
  Readonly<{
    timestamp: number;
  }>
>;

It is a SyntheticEvent, and that's defined in React types. This one is quite confusing [source]

interface SyntheticEvent<T = Element, E = Event> extends BaseSyntheticEvent<E, EventTarget & T, EventTarget> {}

Note how the order of the generics are swapped on the BaseSyntheticEvent.

Then, looking at BaseSyntheticEvent we have this [source]:

interface BaseSyntheticEvent<E = object, C = any, T = any> {
  nativeEvent: E;
  ...
}

So, the type of the nativeEvent is E, the first generic of BaseSyntheticEvent, which is the second generic of SyntheticEvent. In the Event declaration we only send one, so we're not setting the type for the event.nativeEvent.

I'm guessing not defining a type for nativeEvent is the reason my TS is picking the wrong nativeEvent type, but I actually have no idea about that.

All 10 comments

hello and thanks for reporting, this looks weird 馃
The TS typings do not use timeStamp (https://github.com/react-native-datetimepicker/datetimepicker/blob/master/src/index.d.ts) and neither does the codebase (https://github.com/react-native-datetimepicker/datetimepicker/search?q=timestamp)

what is the exact error message you're getting? Thanks

It also looks weird to me, but I'm not too familiar with TS types...

The error I get is this
image

Just in case you need it in typing:

Property 'timestamp' does not exist on type 'Event'. Did you mean 'timeStamp'?ts(2551)
lib.dom.d.ts(5325, 14): 'timeStamp' is declared here.

Interesting.

I've gone a bit more in depth looking those links, and then came back to my code and drilled down the types with cmd + click. I think I've found something.

I'm going to detail all the steps just to make sure I didn't jump to the wrong place by mistake, but that's going to make this a bit lengthy. I'll add a TL;DR version.

TL;DR

I think the Event declared here should be sending the Readonly type as the second generic, and probably unknown as the first. Something like this

export type Event = SyntheticEvent<
  unknown,
  Readonly<{
    timestamp: number;
  }>
>;

Doing that actually fixes the error the linter was giving me.

If, after reading the long version, you think that's a valid solution for this issue, I'll be happy to open a PR with the change.

Long version

I started from this definition [source]

declare const RNDateTimePicker: FC<
  IOSNativeProps | AndroidNativeProps | WindowsNativeProps
>;

Then I went to find onChange on IOSNativeProps | AndroidNativeProps | WindowsNativeProps. All of them extend BaseProps. Inside BaseProps there is DateOptions [source], DateOptions extends BaseOptions [source] and inside BaseOptions I found the onChange declaration [source].

onChange?: (event: Event, date?: Date) => void;

That Event type is what we're talking about, and it's declared at the top of the file [source]

export type Event = SyntheticEvent<
  Readonly<{
    timestamp: number;
  }>
>;

It is a SyntheticEvent, and that's defined in React types. This one is quite confusing [source]

interface SyntheticEvent<T = Element, E = Event> extends BaseSyntheticEvent<E, EventTarget & T, EventTarget> {}

Note how the order of the generics are swapped on the BaseSyntheticEvent.

Then, looking at BaseSyntheticEvent we have this [source]:

interface BaseSyntheticEvent<E = object, C = any, T = any> {
  nativeEvent: E;
  ...
}

So, the type of the nativeEvent is E, the first generic of BaseSyntheticEvent, which is the second generic of SyntheticEvent. In the Event declaration we only send one, so we're not setting the type for the event.nativeEvent.

I'm guessing not defining a type for nativeEvent is the reason my TS is picking the wrong nativeEvent type, but I actually have no idea about that.

Can confirm that this seems to fix the issue. This currently breaks our build process because it fails testing.

All tests pass if unknown is made the first type in Event = SyntheticEvent...

hi! if something fixes an issue for 2 people already, I consider that a trustworthy fix. Please be so kind and open a PR with the fix, thank you!

Do I need to update both index.js and index.d.ts here?
Anything special to run before I send the PR up?

also, what is the best way to test this fix is actually working before we merge it in? Any ideas?

@lexicalninja is it fixed? facing same issue.

import RNDateTimePicker, { AndroidEvent, Event } from '@react-native-community/datetimepicker'
.....

 onChange={(event: Event | AndroidEvent, date?: Date): void => {
     ...
})

This is how I got it to work if it helps

Was this page helpful?
0 / 5 - 0 ratings

Related issues

mustafaskyer picture mustafaskyer  路  5Comments

ArturDabrowski picture ArturDabrowski  路  3Comments

wesff picture wesff  路  3Comments

Kamalnrf picture Kamalnrf  路  3Comments

danlugo92 picture danlugo92  路  5Comments