Convert packages/react storybook to typescript. Convert checkbox story to tsx #12000

mbarrer · 2022-08-23T16:51:52Z

Changes have been made to allow the use of typescript within packages/react so that storybook stories can start to be written in typescript. As part of this change, the main.js for storybook has now been converted to main.ts and has some initial typings. Typing files have been added for Checkbox that we're sourced from the DefinitelyTyped repo and adjusted for carbon v11

Changelog

New

"typescript": "^4.7.4"
"@typescript-eslint/eslint-plugin": "^5.30.5" and "@typescript-eslint/parser": "^5.34.0" for linting
"ts-node": "^10.9.1" for supporting imports in storybook main.ts
typings/shared.d.ts common types file pulled from DefinitielyTyped
components/Checkbox/Checkbox.d.ts new typing file for Checkbox (adjusted DT typings)
custom.d.ts ambient module to allow imports of .mdx files in .tsx
tsconfig.json

Changed

update mini-css-extract-plugin to support native ts typings
converted .storybook/main.js to typescript and allow stories to be written in typescript
converted components/Checkbox/Checkbox.stories.js to tsx using the new typings
Added linting for .ts, .tsx to eslint-config-carbon/plugins/react.js

Testing / Reviewing

In order to test:

fresh yarn install
new build
launch storybook and verify that the Checkbox story is working and has the same functionality as it had before

Add typescript dependency to carbon/react. Convert storybook main.js and Checkbox story to ts.

fix: change tsconfig module to commonjs

github-actions · 2022-08-23T16:52:09Z

DCO Assistant Lite bot All contributors have signed the DCO.

netlify · 2022-08-23T17:39:56Z

✅ Deploy Preview for carbon-components-react ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`cfef450`
🔍 Latest deploy log	https://app.netlify.com/sites/carbon-components-react/deploys/6328a237aeaec6000847ed06
😎 Deploy Preview	https://deploy-preview-12000--carbon-components-react.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

netlify · 2022-08-23T17:39:56Z

✅ Deploy Preview for carbon-elements ready!

Name	Link
🔨 Latest commit	`cfef450`
🔍 Latest deploy log	https://app.netlify.com/sites/carbon-elements/deploys/6328a237f4719800083d252e
😎 Deploy Preview	https://deploy-preview-12000--carbon-elements.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

mbarrer · 2022-08-23T17:52:46Z

I have read the DCO document and I hereby sign the DCO.

packages/react/.storybook/main.ts

packages/react/src/typings/shared.d.ts

tay1orjones · 2022-08-23T20:17:43Z

@mbarrer Are you using Yarn v1 by chance? The project is configured to need yarn@3.2.1. I believe this is why the deploy previews are failing. I think if you update/install to v3.2.1 and re-run yarn install from the root things should start working.

Since you added new dependencies there should also be some additions to .yarn/cache as well for you to commit and push up in addition to the yarn.lock

mbarrer · 2022-08-23T21:03:48Z

Ah yes I have yarn 1.22.18 installed, I can update this on my end and run a fresh yarn install. I didnt see any .yarn/cache changes but that may just be because my version is so old

tay1orjones · 2022-08-24T19:33:41Z

@mbarrer Hey so out of curiosity I pulled down this PR, reset yarn.lock to what's currently in main, and then re-ran yarn install. This pulled the changed files down from ~1800+ to just 26. I think the snafu with yarn v1 inadvertently reset all the hashes on the yarn cache, which is why it was such a huge change set.

Anyway, should be fixed now 👍

metonym · 2022-08-24T20:26:52Z

packages/react/.storybook/main.ts

-    const babelLoader = config.module.rules.find((rule) => {
-      return rule.use.some(({ loader }) => {
+  // TODO: Fix typings for this function
+  webpack(config: any) {


If statically typing this is desired, there are two options:

Only use Configuration type from Webpack

import type { Configuration } from 'webpack'; ... webpack(config: Configuration) {

Type the entire config using StorybookConfig

import type { StorybookConfig } from '@storybook/core-common'; const config: StorybookConfig = { /** */ } module.exports = config;

Yep nice call, there was a few other changes needed in the body of the config to account for the union types used in the webpackFinal prop. The use of the typings also caught the fact that the webpack property should have actually been renamed to webpackFinal according to the storybook 6.5 docs

mbarrer · 2022-08-26T17:00:38Z

Looks like the build task also needs to be adjusted to output a main.js file for use in netlify, will be taking a look at this

erifsx

This looks good to me.

I was concerned about what would happen if we released this without all the other types, or at least empty export declare let XX = unknown declarations for everything so I looked in the compiled lib and es folders and it doesn't look like it's actually shipping the types there yet and carbon doesn't ship the src folder. When we want to "go live" with the feature, I believe they'll need to be copied to the appropriate place along with an index.d.ts file.

Given all that, this looks like a great way to dark release our work.

mbarrer · 2022-08-30T22:59:59Z

Just letting you guys know that I haven't forgot about this PR, have just been busy this week with story work. I'll aim to have the build issue resolved by the end of the week

falsepopsky · 2022-08-31T01:50:54Z

packages/react/.storybook/main.ts

@@ -4,19 +4,17 @@
 * This source code is licensed under the Apache-2.0 license found in the
 * LICENSE file in the root directory of this source tree.
 */
+import fs from 'fs';


I think it would be a good practice to put node: before using each node core modules/lib, etc.

example:

import fs from 'node:fs' fs Docs
import path from 'node:path' Path docs

oh cool I didnt know you could reference these like this. good to know!

mbarrer · 2022-09-02T18:11:38Z

Seems like the issue I was hitting was something that @erifsx brought up in a thread on the workgroup slack. We found that we didnt need ts-node to get things running initially but for some reason this is now required in order to run the main.ts. Adding this to the dev dependancies seems to resolve the issues with netlify

jharvey10 · 2022-09-09T17:31:10Z

config/eslint-config-carbon/plugins/react.js

+    {
+      files: ['*.ts', '*.tsx'],
+      plugins: ['@typescript-eslint'],
+      extends: ['plugin:@typescript-eslint/recommended'],
+    },


Assuming carbon is using the no-unused-vars JS rule, you'll want to turn that off in favor of the typescript-eslint one. Here's an example

jharvey10 · 2022-09-09T17:36:19Z

packages/react/src/components/Checkbox/Checkbox.Skeleton.d.ts

@@ -0,0 +1,7 @@
+import { FunctionComponent, HTMLAttributes } from 'react';


Missing copyright statement

jharvey10 · 2022-09-09T17:36:28Z

packages/react/src/components/Checkbox/Checkbox.d.ts

@@ -0,0 +1,52 @@
+import * as React from 'react';


Missing copyright statement

jharvey10 · 2022-09-09T17:42:22Z

packages/react/src/components/Checkbox/index.d.ts

@@ -0,0 +1,5 @@
+import Checkbox from './Checkbox';


Missing copyright statement

jharvey10 · 2022-09-09T17:45:02Z

packages/react/src/components/Notification/Notification-test.js

@@ -1,3 +1,4 @@
+/* eslint-disable jest/no-standalone-expect */


Which part of this file is violating this rule? Should it instead be updated to conform to it?

Hmm not sure why this was added, removing does not seem to cause any warnings to show

jharvey10 · 2022-09-09T22:37:00Z

packages/react/src/custom.d.ts

@@ -0,0 +1,5 @@
+// Ambient module to allow the importing of .mdx files in storybook .tsx files
+declare module '*.mdx' {
+  const content: any;


We should definitely aim for zero warnings. This should either be unknown or if working around the lack of explicit any is too onerous to devs (since this is a gradual conversion), perhaps we consider disabling the no-explicit-any rule to start.

Good point, i should have changed this to unknown

mbarrer · 2022-09-19T16:56:51Z

@jdharvey-ibm updated PR based off your recent comments

tay1orjones · 2022-11-01T12:50:53Z

Closing in favor of #12222

mbarrer and others added 11 commits July 6, 2022 15:29

feat: tsconfig and eslint to support typescript

33c1b71

feat: convert main.js storybook to ts. Convert checkbox story to tsx

38dd38d

feat: fix linting errors

ec89107

chore(merge): merge branch main

1566a5a

chore: update yarn.lock

b79c8f1

Merge pull request #1 from mbarrer/ts-storybook

d896f6a

Add typescript dependency to carbon/react. Convert storybook main.js and Checkbox story to ts.

fix: change tsconfig module to commonjs

18c8b76

Merge pull request #2 from mbarrer/ts-storybook

056660d

fix: change tsconfig module to commonjs

Merge branch 'main' of github.com:carbon-design-system/carbon

a10ab9a

refactor: add comments to props and typing for playground args

d034edd

chore: add comment for ambient module

760ef50

mbarrer requested a review from a team as a code owner August 23, 2022 16:51

mbarrer requested review from tay1orjones and dakahn August 23, 2022 16:51

tay1orjones requested a review from a team August 23, 2022 17:57

sierrawetmore reviewed Aug 23, 2022

View reviewed changes

packages/react/.storybook/main.ts Outdated Show resolved Hide resolved

sierrawetmore approved these changes Aug 23, 2022

View reviewed changes

packages/react/src/typings/shared.d.ts Outdated Show resolved Hide resolved

tay1orjones requested a review from a team August 23, 2022 20:18

mbarrer and others added 5 commits August 23, 2022 16:22

chore: rebuild with yarn 3.2.1

1e2fb9a

chore: remove commented out types

cb16af7

Merge branch 'main' of github.com:carbon-design-system/carbon

551d6ac

chore: add missing eslint parser devDependancy

1af0750

chore(yarn): reset yarn.lock and update yarn cache

3f69616

metonym reviewed Aug 24, 2022

View reviewed changes

mbarrer added 3 commits August 26, 2022 10:21

refactor: add StorybookConfig typing to config object

43e3966

Merge branch 'main' of github.com:mbarrer/carbon

0bffe0d

chore: remove TODO

d188dd6

tay1orjones requested a review from erifsx August 29, 2022 12:41

erifsx approved these changes Aug 29, 2022

View reviewed changes

falsepopsky reviewed Aug 31, 2022

View reviewed changes

mbarrer added 2 commits September 2, 2022 12:52

refactor: add ts-node and update mini-css-extract-plugin

871d4d1

chore: update node imports

b3a4665

mbarrer added 2 commits September 2, 2022 13:59

chore: run yarn dedupe

af3541b

Merge branch 'main' of github.com:carbon-design-system/carbon

6270409

metonym approved these changes Sep 6, 2022

View reviewed changes

tay1orjones requested a review from jharvey10 September 8, 2022 17:26

dakahn removed their request for review September 8, 2022 21:24

jharvey10 reviewed Sep 9, 2022

View reviewed changes

mbarrer added 4 commits September 19, 2022 11:52

chore: review comments

c05f566

chore: update @typescript/eslint-plugin

1bbdc25

refactor: remove any type from main.ts

51765ae

Merge branch 'main' of github.com:carbon-design-system/carbon

a55f812

chore: run yarn dedupe

cfef450

tay1orjones closed this Nov 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert packages/react storybook to typescript. Convert checkbox story to tsx #12000

Convert packages/react storybook to typescript. Convert checkbox story to tsx #12000

mbarrer commented Aug 23, 2022 •

edited

Loading

github-actions bot commented Aug 23, 2022 •

edited

Loading

netlify bot commented Aug 23, 2022 •

edited

Loading

netlify bot commented Aug 23, 2022 •

edited

Loading

mbarrer commented Aug 23, 2022

tay1orjones commented Aug 23, 2022 •

edited

Loading

mbarrer commented Aug 23, 2022 •

edited

Loading

tay1orjones commented Aug 24, 2022

metonym Aug 24, 2022

mbarrer Aug 26, 2022

mbarrer commented Aug 26, 2022

erifsx left a comment •

edited

Loading

mbarrer commented Aug 30, 2022

falsepopsky Aug 31, 2022

mbarrer Aug 31, 2022

mbarrer commented Sep 2, 2022

jharvey10 Sep 9, 2022

jharvey10 Sep 9, 2022

jharvey10 Sep 9, 2022

jharvey10 Sep 9, 2022

jharvey10 Sep 9, 2022

mbarrer Sep 19, 2022

jharvey10 Sep 9, 2022

mbarrer Sep 19, 2022

mbarrer commented Sep 19, 2022

tay1orjones commented Nov 1, 2022

		@@ -0,0 +1,7 @@
		import { FunctionComponent, HTMLAttributes } from 'react';

		@@ -1,3 +1,4 @@
		/* eslint-disable jest/no-standalone-expect */

Convert packages/react storybook to typescript. Convert checkbox story to tsx #12000

Convert packages/react storybook to typescript. Convert checkbox story to tsx #12000

Conversation

mbarrer commented Aug 23, 2022 • edited Loading

Changelog

Testing / Reviewing

github-actions bot commented Aug 23, 2022 • edited Loading

netlify bot commented Aug 23, 2022 • edited Loading

✅ Deploy Preview for carbon-components-react ready!

netlify bot commented Aug 23, 2022 • edited Loading

✅ Deploy Preview for carbon-elements ready!

mbarrer commented Aug 23, 2022

tay1orjones commented Aug 23, 2022 • edited Loading

mbarrer commented Aug 23, 2022 • edited Loading

tay1orjones commented Aug 24, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbarrer commented Aug 26, 2022

erifsx left a comment • edited Loading

Choose a reason for hiding this comment

mbarrer commented Aug 30, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbarrer commented Sep 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbarrer commented Sep 19, 2022

tay1orjones commented Nov 1, 2022

mbarrer commented Aug 23, 2022 •

edited

Loading

github-actions bot commented Aug 23, 2022 •

edited

Loading

netlify bot commented Aug 23, 2022 •

edited

Loading

netlify bot commented Aug 23, 2022 •

edited

Loading

tay1orjones commented Aug 23, 2022 •

edited

Loading

mbarrer commented Aug 23, 2022 •

edited

Loading

erifsx left a comment •

edited

Loading