Port over minimal TS configs, deps and port Pane #1590
Conversation
|
Pulled this into app and there's definitely something jacked up with the Pane types - going to do some further investigation! |
|
After updating the type cast of the memoized Pane and re-adding the |
✅ Deploy Preview for evergreen-storybook ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
| "postpublish": "branch=$(git rev-parse --symbolic-full-name --abbrev-ref HEAD); if [[ $branch == \"master\" ]]; then yarn update-docs || yarn deploy-docs; else echo \"not on main branch, skipping docs deploy\"; fi;", | ||
| "prepublishOnly": "rm -rf esm commonjs umd && yarn build", | ||
| "prepublishOnly": "rm -rf esm commonjs umd types && yarn build", | ||
| "release": "np", | ||
| "size": "size-limit", |
There was a problem hiding this comment.
may want to add a tsc typecheck command and add it as an initial step in the build
There was a problem hiding this comment.
I think the yarn build-types or yarn build-commonjs steps will fail if there are TS errors since they use tsc, but I can add a more explicit check that runs tsc --noEmit before any of them 👍
yarn build
yarn run v1.22.5
$ yarn generate-icons && concurrently --names 'commonjs,esm,umd,codemods,types' 'yarn build-commonjs' 'yarn build-esm' 'yarn build-umd' 'yarn build-codemods' 'yarn build-types'
$ ./tools/generate-icons.js
$ tsc --project tsconfig.commonjs.json
$ BABEL_ENV=esm babel src --out-dir esm --ignore '**/stories','**/test','**/__tests__','**/types' --extensions '.js,.ts,.tsx'
$ rollup -c
$ cd codemods && yarn build
$ tsc --project tsconfig.types.json
$ yarn clean
$ rm -rf dist
$ tsc
[esm] Browserslist: caniuse-lite is outdated. Please run:
[esm] npx browserslist@latest --update-db
[esm]
[esm] Why you should do it regularly:
[esm] https://github.com/browserslist/browserslist#browsers-data-updating
[umd]
[umd] src/index.ts → umd/evergreen.js...
[codemods] yarn build-codemods exited with code 0
[esm] Successfully compiled 809 files with Babel (13039ms).
[esm] yarn build-esm exited with code 0
[commonjs] src/layers/src/Pane.tsx(106,6): error TS2339: Property 'foo' does not exist on type '<T extends ElementType<any> = "div">(props: PaneProps<T>) => Element'.
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
[commonjs] yarn build-commonjs exited with code 2
[umd] Browserslist: caniuse-lite is outdated. Please run:
[umd] npx browserslist@latest --update-db
[umd]
[umd] Why you should do it regularly:
[umd] https://github.com/browserslist/browserslist#browsers-data-updating
[types] src/layers/src/Pane.tsx(106,6): error TS2339: Property 'foo' does not exist on type '<T extends ElementType<any> = "div">(props: PaneProps<T>) => Element'.
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
[types] yarn build-types exited with code 2
[umd] (!) Plugin typescript: @rollup/plugin-typescript TS2339: Property 'foo' does not exist on type '<T extends ElementType<any> = "div">(props: PaneProps<T>) => Element'.
[umd] src/layers/src/Pane.tsx: (106:6)
[umd]
[umd] 106 Pane.foo = 123;
[umd] ~~~
[umd]
[umd] src/layers/src/Pane.tsx: (106:6)
[umd]
[umd] 106 Pane.foo = 123;
[umd] ~~~
[umd]
[umd] src/layers/src/Pane.tsx: (106:6)
[umd]
[umd] 106 Pane.foo = 123;
[umd] ~~~
[umd]
[umd] src/layers/src/Pane.tsx: (106:6)
[umd]
[umd] 106 Pane.foo = 123;
[umd] ~~~
[umd]
[umd] created umd/evergreen.js in 29s
[umd]
[umd] src/index.ts → umd/evergreen.min.js...
[umd] (!) Plugin typescript: @rollup/plugin-typescript TS2339: Property 'foo' does not exist on type '<T extends ElementType<any> = "div">(props: PaneProps<T>) => Element'.
[umd] src/layers/src/Pane.tsx: (106:6)
[umd]
[umd] 106 Pane.foo = 123;
[umd] ~~~
[umd]
[umd] created umd/evergreen.min.js in 22.5s
[umd] yarn build-umd exited with code 0
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.| export type PaneProps<T extends React.ElementType = 'div'> = PolymorphicBoxProps<T, PaneOwnProps> | ||
|
|
||
| // Temporary type alias to satisfy TS, may be extracted to a shared types directory later | ||
| type Elevation = 0 | 1 | 2 | 3 | 4 |
There was a problem hiding this comment.
maybe useful here if we upgrade: https://catchts.com/range-numbers
| const Pane = memo(forwardRef(_Pane)) as <T extends React.ElementType = 'div'>(props: PaneProps<T>) => JSX.Element | ||
|
|
||
| // @ts-expect-error TS(2339): Property 'propTypes' does not exist on type '<E ex... Remove this comment to see the full error message | ||
| Pane.propTypes = { |
There was a problem hiding this comment.
curious why we are leaving this?
There was a problem hiding this comment.
We actually parse the propTypes from the component src files for the /:component/props page on our doc site (i.e. https://evergreen.segment.com/components/file-uploader/props), so I'm keeping them in until more time can be dedicated to a solution that reads the ts definitions instead
I ran into this during my massive 1-pass-port and tried swapping out react-docgen with something like https://github.com/styleguidist/react-docgen-typescript, but I think that came with its own issues/upgrades that were needed first.
Overview
This was bit of a stretch goal of mine for V7 - bringing TypeScript into the project and kicking off the port! Instead of doing a massive one-pass refactor from JS -> TS like #1467, I pulled out the parts of the migration utils I wrote and reworked them to act on one component at a time, and (for the most part) be re-runnable without side effects if the component has already been ported.
In the interest of keeping this PR smaller and not littering the repo with temporary codemod scripts that aren't necessarily battle-tested yet, I kept them out of this PR. I may break out another repo or just keep a branch alive that has the utilities to use when I'm porting over more components.
Summary of changes:
package.jsonto the generatedindex.d.tsI'm not convinced bundling of
@types/reactas a production dependency is necessary, but it does seem like@types/react-transition-groupis necessary (or theOverlayPropsbreak for consumers), so I've removed the former.Screenshots (if applicable)
Documentation