Migrate remaining components to TypeScript #2065
Conversation
🦋 Changeset detectedLatest commit: 26db23e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
connor-baer
force-pushed
the
typescript-all-the-things
branch
from
f7111b5
to
4c7012a
Compare
Codecov Report
@@ Coverage Diff @@
## next #2065 +/- ##
==========================================
- Coverage 97.10% 96.81% -0.30%
==========================================
Files 250 248 -2
Lines 22392 22383 -9
Branches 2105 2117 +12
==========================================
- Hits 21743 21669 -74
- Misses 641 703 +62
- Partials 8 11 +3
|
connor-baer
force-pushed
the
typescript-all-the-things
branch
from
4c7012a
to
26db23e
Compare
| .circuit-0 { | ||
| display: block; | ||
| position: relative; | ||
| overflow: hidden; | ||
| height: auto; | ||
| width: 100%; | ||
| } |
There was a problem hiding this comment.
I removed these default styles when no aspect-ratio is provided. The result should be the same.
| {this.getDateRangePreview()} | ||
| </Tag> | ||
| {isOpen && ( | ||
| <CalendarWrap> | ||
| <RangePickerController | ||
| initialVisibleMonth={null} |
There was a problem hiding this comment.
Added this to make TypeScript happy. The component works the same with and without 🤷♂️
| onClick={this.handleTagClick} | ||
| > | ||
| {this.getDateRangePreview()} | ||
| </Tag> | ||
| {isOpen && ( | ||
| <CalendarWrapper> | ||
| {/* @ts-expect-error This worked before the component was converted to TypeScript */} |
There was a problem hiding this comment.
I didn't bother fixing the types here since the component will soon be replaced.
| @@ -64,17 +112,17 @@ const animationStyles = ({ | |||
| animation-name: ${animationName}; | |||
| animation-duration: ${animationDuration}ms; | |||
| animation-fill-mode: forwards; | |||
| animation-timing-function: ${theme.transitions.easeInOutCubic}; | |||
There was a problem hiding this comment.
This transition doesn't exist on the theme (anymore?).
There was a problem hiding this comment.
Wow! This is great, thanks for doing it. It's true that we were in a bit of a tricky situation, with legacy components that we didn't bother migrating to TS initially, but also blocked by these very components when cleaning up/improving our stack.
I haven't looked at the full diff very closely since many of these components are either legacy or have low usage. There will definitely be type errors in consumer apps, but we can probably @ts-ignore many of them during the migration—since these components should eventually be replaced (or rebuilt) anyway.
In a word: fantastisch
packages/circuit-ui/components/CalendarTagTwoStep/CalendarTagTwoStep.tsx
Show resolved
Hide resolved
Completes #516.
Purpose
While working on #2061, I encountered an issue with the remaining JavaScript files. Files that contain JSX need to use the
.jsxextension. ESM requires imports to include the filename and extension. However, once TypeScript transpiles the files using the"jsx": "react-jsx"config option, it uses the.jsfile extension on disk but doesn't update the imports to match.This isn't an issue for TypeScript files, where the final extension should be used for imports (i.e.,
.js).Approach and changes
prop-typesdependencyDefinition of done