Skip to content

Move flow types to .flow typedef files#1509

Merged
kof merged 33 commits intomasterfrom
move-flow-to-typedefs
Sep 5, 2021
Merged

Move flow types to .flow typedef files#1509
kof merged 33 commits intomasterfrom
move-flow-to-typedefs

Conversation

@kof
Copy link
Copy Markdown
Member

@kof kof commented May 5, 2021

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

declare export default class PluginsRegistry {
plugins: {|
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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 kof force-pushed the move-flow-to-typedefs branch from e7efd5c to 117dcb8 Compare July 16, 2021 11:43
@kof
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

2 participants