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

fix: Update incorrect SwitchChangeEvent type #34931

Closed
wants to merge 1 commit into from

Conversation

retyui
Copy link
Contributor

@retyui retyui commented Oct 10, 2022

Summary

I noticed that typescript type for onChange event of <Switch/> was incorrect

<Switch
  onChange={(event) => {
    // TS
    event.value; // boolean
    event.nativeEvent.value; //TS2339: Property 'value' does not exist on type 'Event'.
    // JS
    console.log(event.nativeEvent); // {value:false,target:87}
    console.log(event.value); // undefined
  }}
/>

Changelog

[General] [Changed] - Typescript: update incorrect SwitchChangeEvent type

Test Plan

...

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 10, 2022
@retyui retyui force-pushed the fix-switch-on-change-event branch 2 times, most recently from a868181 to c547f86 Compare October 10, 2022 17:38
@analysis-bot
Copy link

analysis-bot commented Oct 10, 2022

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,768,225 -4,111
android hermes armeabi-v7a 7,169,438 -3,990
android hermes x86 8,081,465 -4,128
android hermes x86_64 8,053,107 -4,051
android jsc arm64-v8a 9,629,253 -4,258
android jsc armeabi-v7a 8,393,784 -4,145
android jsc x86 9,578,678 -4,295
android jsc x86_64 10,171,827 -4,184

Base commit: 14456e7
Branch: main

@skinsshark
Copy link
Contributor

hi @retyui, thanks for the fix! some of the CI tests are failing though, could you take a look?

@facebook-github-bot
Copy link
Contributor

@skinsshark has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@retyui
Copy link
Contributor Author

retyui commented Oct 11, 2022

some of the CI tests are failing thou

@skinsshark nothing critical, I think a retry should fix it:

CI/DS logs...
// android 

* What went wrong:
Execution failed for task ':react-native-gradle-plugin:compileKotlin'.
> Error while evaluating property 'filteredArgumentsMap' of task ':react-native-gradle-plugin:compileKotlin'
   > Could not resolve all files for configuration ':react-native-gradle-plugin:compileClasspath'.
      > Could not download builder-7.3.0.jar (com.android.tools.build:builder:7.3.0)
         > Could not get resource 'https://dl.google.com/dl/android/maven2/com/android/tools/build/builder/7.3.0/builder-7.3.0.jar'.
            > Premature end of Content-Length delimited message body (expected: 8,738,708; received: 6,029,280)
//-----------------------
// ios
 Executed 173 tests, with 0 failures (0 unexpected) in 0.790 (0.845) seconds

Failing tests:
	RNTesterIntegrationTests:
		-[RCTLoggingTests testLogging]
		-[RCTLoggingTests testLogging]

** TEST FAILED **

./IntegrationTests/launchWebSocketServer.sh: line 9: 10888 Terminated: 15          ./websocket_integration_test_server.js
~/react-native
Process terminated.
error Command failed with signal "SIGTERM".

@retyui retyui requested a review from lunaleaps October 11, 2022 07:34
@retyui retyui force-pushed the fix-switch-on-change-event branch 2 times, most recently from 17357ac to 609e1cf Compare October 11, 2022 20:16
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: b33961d
Branch: main

@facebook-github-bot
Copy link
Contributor

@skinsshark has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @retyui in 5dd2f2e.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Oct 12, 2022
mohitcharkha pushed a commit to mohitcharkha/react-native that referenced this pull request Oct 17, 2022
Summary:
I noticed that typescript type for `onChange`  event of `<Switch/>` was incorrect

```tsx
<Switch
  onChange={(event) => {
    // TS
    event.value; // boolean
    event.nativeEvent.value; //TS2339: Property 'value' does not exist on type 'Event'.
    // JS
    console.log(event.nativeEvent); // {value:false,target:87}
    console.log(event.value); // undefined
  }}
/>
```

## Changelog

[General] [Changed] - Typescript: update incorrect `SwitchChangeEvent` type

Pull Request resolved: facebook#34931

Test Plan: ...

Reviewed By: lunaleaps

Differential Revision: D40240552

Pulled By: skinsshark

fbshipit-source-id: 4d39d547778de4ac4dc6c94471f05bfbe157d0e5
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
Summary:
I noticed that typescript type for `onChange`  event of `<Switch/>` was incorrect

```tsx
<Switch
  onChange={(event) => {
    // TS
    event.value; // boolean
    event.nativeEvent.value; //TS2339: Property 'value' does not exist on type 'Event'.
    // JS
    console.log(event.nativeEvent); // {value:false,target:87}
    console.log(event.value); // undefined
  }}
/>
```

## Changelog

[General] [Changed] - Typescript: update incorrect `SwitchChangeEvent` type

Pull Request resolved: facebook#34931

Test Plan: ...

Reviewed By: lunaleaps

Differential Revision: D40240552

Pulled By: skinsshark

fbshipit-source-id: 4d39d547778de4ac4dc6c94471f05bfbe157d0e5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants