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

Road to implicitAny part 4 #4497

Merged
merged 12 commits into from
Dec 25, 2018
Merged

Road to implicitAny part 4 #4497

merged 12 commits into from
Dec 25, 2018

Conversation

henrikra
Copy link
Contributor

@henrikra henrikra commented Dec 24, 2018

What is this?

This PR continues the work on making this project type safe with TypeScript. Goal is that the flag noImplicitAny will be set to true

What this PR does?

  • all files inside adapter folder is valid for noImplicitAny!
  • resolveAssetSource is now imported from the current place where it should. Previously it was imported from legacy location. More info here: https://facebook.github.io/react-native/docs/image#resolveassetsource
  • formatted many files
  • add missing interfaces
  • fix onPressIn prop on TouchablePreview

What's next?

Next I am planning to make same treatment for commands folder and many it valid for noImplicitAny 🎉

@henrikra henrikra changed the title [WIP] Road to implicitAny v3 Road to implicitAny part 4 Dec 24, 2018
@henrikra
Copy link
Contributor Author

@guyca Thx for merging my last PR so quickly! Here is next follow up :D

Copy link
Collaborator

@guyca guyca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great @henrikra !! I noticed that a few unneeded comma character have been added, can you please delete them?

lib/src/adapters/NativeCommandsSender.ts Show resolved Hide resolved
@@ -7,22 +7,24 @@ import {
SearchBarUpdatedEvent,
SearchBarCancelPressedEvent,
PreviewCompletedEvent,
ModalDismissedEvent
ModalDismissedEvent,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we avoid adding comma character where it's avoidable?

@@ -31,39 +33,57 @@ export class NativeEventsReceiver {
return this.emitter.addListener('RNN.AppLaunched', callback);
}

public registerComponentDidAppearListener(callback: (event: ComponentDidAppearEvent) => void): EventSubscription {
public registerComponentDidAppearListener(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets return these method declarations to single line - I think it's readable enough in a single line.

@guyca guyca self-assigned this Dec 25, 2018
@henrikra
Copy link
Contributor Author

I have now done the changes you requested :)

@guyca guyca merged commit 02985c5 into wix:master Dec 25, 2018
vshkl pushed a commit to vshkl/react-native-navigation that referenced this pull request Feb 5, 2020
What is this?
This PR continues the work on making this project type safe with TypeScript. Goal is that the flag noImplicitAny will be set to true

What this PR does?
* all files inside adapter folder is valid for noImplicitAny!
* resolveAssetSource is now imported from the current place where it should. Previously it was imported from legacy location. More info here: https://facebook.github.io/react-native/docs/image#resolveassetsource
* formatted many files
* add missing interfaces
* fix onPressIn prop on TouchablePreview

What's next?
Next I am planning to make same treatment for commands folder and many it valid for noImplicitAny 🎉
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants