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

upkeep: Add TS support for Optional component #298

Merged
merged 9 commits into from
Mar 11, 2024

Conversation

Sidsector9
Copy link
Member

Description of the Change

Closes #

Changelog Entry

Changed - Add TS support toOptional component

Credits

Props @fabiankaegy @Sidsector9

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

Copy link

github-actions bot commented Mar 1, 2024

Size Change: -122 B (0%)

Total Size: 65.6 kB

Filename Size Change
dist/index.js 65.6 kB -122 B (0%)

compressed-size-action

Copy link

cypress bot commented Mar 1, 2024

Passing run #768 ↗︎

0 6 0 0 Flakiness 0

Details:

Merge branch 'upkeep/component-optional' of github.com:10up/block-components int...
Project: 10up Block Components Commit: 355e598048
Status: Passed Duration: 01:31 💡
Started: Mar 11, 2024 9:58 AM Ended: Mar 11, 2024 9:59 AM

Review all test suite changes for PR #298 ↗︎

@@ -1,7 +1,15 @@
import PropTypes from 'prop-types';
Copy link
Member

Choose a reason for hiding this comment

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

I actually think that as part of converting something to TypeScript it makes sense to get rid of all the PropTypes stuff.

export const Optional: React.FC<OptionalProps> = ({
value,
children,
}): boolean | React.ReactNode => {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure we need / want to explicitly type the return type of every function. If something can be inferred I would rather just leave it up to the compiler

@Sidsector9 Sidsector9 changed the title upkeep: Add TS support toOptional component upkeep: Add TS support forOptional component Mar 1, 2024
@Sidsector9 Sidsector9 changed the title upkeep: Add TS support forOptional component upkeep: Add TS support for Optional component Mar 1, 2024
Co-authored-by: Fabian Kägy <mail@fabian-kaegy.de>
Comment on lines 23 to 25
Optional.defaultProps = {
value: '',
};
Copy link
Member

Choose a reason for hiding this comment

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

This can be removed now that we have the default value inline

@fabiankaegy fabiankaegy merged commit d78f8c2 into develop Mar 11, 2024
7 of 8 checks passed
@fabiankaegy fabiankaegy deleted the upkeep/component-optional branch March 11, 2024 11:58
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