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

Use TS strict mode in a11y addon sources #9180

Merged
merged 12 commits into from Jan 11, 2020
Merged

Conversation

gaetanmaisse
Copy link
Member

What I did

Use TS strict mode in a11y addon sources. But why?

  • strict mode is included in the default tsconfig.json generated by tsc CLI
  • Stricter rules help to find bugs faster than ever, for instance I found and fix fix(a11y): fix highlight of selected blindness color filter #9179 by using some of the rules I activated in tsconfig.json
  • Some of our users are using TS in strict mode and are expecting that the types we are exposing are working in strict mode (e.g. Storybook 5.2 has issues with TypeScript types #8233)
  • noUnusedLocals put in light the fact that some variables were not used for at least 8 months but still there and still requiring maintenance.

How to test

  • All tests should pass
  • As it is refactoring, everything should work like before

@gaetanmaisse gaetanmaisse added maintenance User-facing maintenance tasks addon: a11y typescript labels Dec 18, 2019
@vercel
Copy link

vercel bot commented Dec 18, 2019

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/storybook/monorepo/ko1hjbvxn
🌍 Preview: https://monorepo-git-a11y-improve-ts-conf.storybook.now.sh

@vercel vercel bot temporarily deployed to staging December 18, 2019 20:35 Inactive
@gaetanmaisse gaetanmaisse marked this pull request as ready for review December 18, 2019 21:26
@ndelangen ndelangen added this to the 6.0.0 milestone Dec 21, 2019
@ndelangen ndelangen self-assigned this Dec 21, 2019
@ndelangen ndelangen changed the base branch from next to next-6.0.0 December 21, 2019 23:26
Copy link
Member

@ndelangen ndelangen left a comment

Choose a reason for hiding this comment

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

How does this relate to #8883?

@gaetanmaisse
Copy link
Member Author

gaetanmaisse commented Dec 24, 2019

@ndelangen I just take a look at #8883, there will be only minor conflicts because this PR focus on refactoring/cleaning some TS (strict mode) issues and not deeply modifying how the addon works. #8883 can be merged (but not immediately as we must wait for 6.0 I guess, just see that you updated the base branch for next-6.0.0), then just tag me and I will fix the conflicts with this branch. 🎄🎅

@ndelangen ndelangen merged commit d216bb6 into next-6.0.0 Jan 11, 2020
@ndelangen ndelangen deleted the a11y-improve-ts-conf branch January 11, 2020 00:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addon: a11y maintenance User-facing maintenance tasks typescript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants