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

[PLAY-102] Enable and Configure Typescript in Playbook #1759

Merged
merged 3 commits into from Feb 15, 2022

Conversation

thestephenmarshall
Copy link
Contributor

@thestephenmarshall thestephenmarshall commented Feb 2, 2022

Breaking Changes

No

Runway Ticket URL

How to test this

Run playbook locally

Checklist:

  • LABELS Add a label: enhancement, bug, improvement, new kit, deprecated, or breaking. See Changelog & Labels for details.
  • DEPLOY Please add the Milano label when you are ready for a review.
  • SCREENSHOT Please add a screen shot or two.
  • SPECS Please cover your changes with specs.
  • READ DOCS Please make sure you have read and understand the Playbook Release Process

@thestephenmarshall thestephenmarshall force-pushed the typescript-webpack branch 2 times, most recently from 1ae560d to 80e5117 Compare February 2, 2022 00:28
@thestephenmarshall thestephenmarshall changed the title WIP: TS in Playbook Enable and Configure Typescript in Playbook Feb 2, 2022
@thestephenmarshall thestephenmarshall changed the title Enable and Configure Typescript in Playbook [PLAY-102] Enable and Configure Typescript in Playbook Feb 2, 2022
@thestephenmarshall thestephenmarshall added typescript milano 20 MAX - Deploy this PR to a review environment via Milano and removed wip labels Feb 2, 2022
@app-milano app-milano bot temporarily deployed to pr1759 February 3, 2022 21:04 Inactive
@thestephenmarshall thestephenmarshall added the breaking Indicates that a breaking change was introduced (USED IN CHANGELOG) label Feb 3, 2022
@app-milano app-milano bot temporarily deployed to pr1759 February 3, 2022 21:41 Inactive
@thestephenmarshall thestephenmarshall force-pushed the typescript-webpack branch 2 times, most recently from 3d9123f to c6d2b1e Compare February 11, 2022 17:09
@thestephenmarshall thestephenmarshall removed the breaking Indicates that a breaking change was introduced (USED IN CHANGELOG) label Feb 14, 2022
@@ -106,6 +106,7 @@ const Button = (props: ButtonPropTypes) => {
className={css}
href={link}
id={id}
rel="noreferrer"
Copy link
Contributor

Choose a reason for hiding this comment

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

is this used for SEO?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is, however, it is also part of the linter settings. We may choose later to remove this however it does serve 3rd party consumers well.

export type GlobalProps = BorderRadius & Cursor & Dark & Display & LineHeight & Margin & MaxWidth & NumberSpacing & Padding & Shadow & ZIndex

// Prop categories
const PROP_CATEGORIES: {[key:string]: (props: {[key: string]: any}) => string} = {
Copy link
Contributor

Choose a reason for hiding this comment

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

is the main advantage here that you are able to specify the datatype to avoid potential errors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Absolutely! Now we can provide developers an expected and auto-documented API for these global props. TS will prevent any out-of-bounds prop values as they type. 😎

Copy link
Contributor

@RobGentile17 RobGentile17 left a comment

Choose a reason for hiding this comment

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

this is a very interesting update it seems like the main advantage is by strictly defining datatypes to avoid future problems

@thestephenmarshall thestephenmarshall merged commit 51e3dce into master Feb 15, 2022
@thestephenmarshall thestephenmarshall deleted the typescript-webpack branch February 15, 2022 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
milano 20 MAX - Deploy this PR to a review environment via Milano typescript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants