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

Move flow types to .flow typedef files #1509

Merged
merged 33 commits into from
Sep 5, 2021
Merged

Move flow types to .flow typedef files #1509

merged 33 commits into from
Sep 5, 2021

Conversation

kof
Copy link
Member

@kof kof commented May 5, 2021

@kof kof mentioned this pull request May 5, 2021
}

declare export default class PluginsRegistry {
plugins: {|
Copy link
Member

Choose a reason for hiding this comment

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

Would we even need to declare this as I wouldn't consider this to be a "public" API and shouldn't really be accessed from the outside?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, was my question as well, since typings are only used from outside, I am not sure internally used types provide any additional type safety, unless used from outside, but thats not going to happen

Copy link
Member

Choose a reason for hiding this comment

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

I think it should be removed as long as we only want to use these types for the outside and not for internal type checking

Copy link
Member Author

Choose a reason for hiding this comment

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

Just tried, can't remove completely all private fields from .flow, it's still gonna be a mixture of public apis and internal once, otherwise it won't compile

Copy link
Member Author

Choose a reason for hiding this comment

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

Because some fields are accessed by jss core itself, from other modules, for actual public api we would need to differentiate that and the access from public apis

@kof
Copy link
Member Author

kof commented Jul 17, 2021

I finally finished moving all flow types in all packages, once this is merged, nothing stops us from using ts

@kof kof merged commit dbef5de into master Sep 5, 2021
@kof kof deleted the move-flow-to-typedefs branch September 5, 2021 10:29
@kof
Copy link
Member Author

kof commented Sep 5, 2021

merged now we can add ts types

kof added a commit that referenced this pull request Sep 5, 2021
@kof
Copy link
Member Author

kof commented Sep 18, 2021

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.

None yet

2 participants