Skip to content

TypeScript: Tests, CSS modules, and dependency updates#178

Merged
mdespuits merged 10 commits intomasterfrom
ts-more-things
May 22, 2020
Merged

TypeScript: Tests, CSS modules, and dependency updates#178
mdespuits merged 10 commits intomasterfrom
ts-more-things

Conversation

@mdespuits
Copy link
Copy Markdown
Contributor

@mdespuits mdespuits commented May 22, 2020

As it turns out, trying to convert existing components and their tests, etc to TypeScript is currently tricky with our outdated dependencies. One of the primary offenders was react-scripts itself. We were running on a 2 year old version. It was worth updating to the latest anyway since it has much better TypeScript support. This meant several things:

  • Upgrading react-scripts changed the matcher for Jest tests. Needed to be *.(spec|test).(ts|js|jsx|tsx) or something along those lines
  • Completely removing the custom css-loader config for Storybook as the react-scripts webpack default configuration works almost without customization. I decided to just rename style.css to {Component}.module.css in keeping with normal conventions for CSS modules.
  • Converted the Responsive.Container to TypeScript to test the problems with the current beta.2 installation in our main app (something to do with auto-generated types with PropTypes)
  • Went ahead and completely converted the Pill component to the updated format (minus the story.js file):
    ❯ tree src/components/Pill
    src/components/Pill
    ├── Addon.tsx
    ├── Pill.module.css
    ├── Pill.test.tsx
    ├── Pill.tsx
    ├── README.md
    ├── index.tsx
    └── story.js

@mdespuits mdespuits changed the title More TypeScript things TypeScript: Tests, CSS modules, and dependency updates May 22, 2020
Copy link
Copy Markdown
Contributor

@pixelbandito pixelbandito left a comment

Choose a reason for hiding this comment

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

This all looks very clean.

My only comment is it looks like 2 eslint rules that are stylistic only are causing a lot of small adjustments.

  • In one case (parens around arrow function single arguments) I actually dislike the new rule.
  • In the other case (space before parens in function (), I don't really care but changing it seems pointless and causes a lot of nitpicky changes from what's currently there.

Otherwise, I see a lot of nice tweaks!

Comment thread src/components/Accordion/Accordion.test.js Outdated
Comment thread package.json
Comment thread package.json Outdated
Comment thread src/components/Avatar/Addon.js
Comment thread src/components/Avatar/index.js Outdated
Comment thread src/components/Collapse/index.js
Comment thread src/components/Callout/index.js
Copy link
Copy Markdown
Contributor

@pixelbandito pixelbandito left a comment

Choose a reason for hiding this comment

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

Thank you!

@mdespuits mdespuits merged commit 1ef5c86 into master May 22, 2020
@mdespuits mdespuits deleted the ts-more-things branch May 22, 2020 18:50
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