fix: additional commas in TsTypeLiteral #1157
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
eventualbuddha
left a comment
eventualbuddha
left a comment
There was a problem hiding this comment.
Thanks! It'd be very useful to have a test that fails before the changes you've made. Mind doing that?
henryqdineen
force-pushed
the
fix-ts-type-literal-comma
branch
2 times, most recently
from
de3213d to
5c660cf
Compare
Collaborator
Author
|
Thanks! I just pushed a commit to add a test following a similar pattern as #1157 Against |
eventualbuddha
approved these changes
Jul 31, 2022
Collaborator
eventualbuddha
left a comment
eventualbuddha
left a comment
There was a problem hiding this comment.
Thanks! Will merge and do a release soon.
eventualbuddha
force-pushed
the
fix-ts-type-literal-comma
branch
from
5c660cf to
8e24cf6
Compare
Collaborator
|
Released in v0.21.2. |
benjamn
added a commit
that referenced
this pull request
Dec 10, 2022
Follow-up to PR #1157 that I caught while trying to regenerate the TypeScript declarations for ast-types, and noticed a bunch of (IMHO reasonable) semicolons going missing.
coderaiser
pushed a commit
to coderaiser/recast
that referenced
this pull request
May 27, 2024
Follow-up to PR benjamn#1157 that I caught while trying to regenerate the TypeScript declarations for ast-types, and noticed a bunch of (IMHO reasonable) semicolons going missing.
jackhsu978
added a commit
to jackhsu978/flow-to-typescript-codemod
that referenced
this pull request
Apr 10, 2025
**What?** Upgrade [recast](https://github.com/benjamn/recast) from `0.20.4` to `0.23.4` for more modern parsers. See [changes](benjamn/[email protected]). As part of the upgrade, the `[email protected]` patch is removed as it is no longer needed. **Why?** When the codemod with `[email protected]` removes comments, the parentheses can be incorrectly removed. Upgrading to `[email protected]` fixes this. For example, the following Flow type: ```js const a = // $FlowFixMe (1 + 1) * 2; ``` was incorrectly converted to: ```ts const a = 1 + 1 * 2; ``` With the new version, it gets correctly converted to: ```ts const a = (1 + 1) * 2; ``` Also, codemod with `[email protected]` keeps the following annotation as is: ```js type Props = { it: string, foo: number }; ```` With [this fix](benjamn/recast#1157) from v0.21.2, the Flow syntax gets correctly converted to the expected TypeScript syntax: ```ts type Props = { it: string; foo: number; }; ``` Lastly, v0.23.4 correctly handles unary expressions by [wrapping unary expressions in parens](benjamn/recast#1361)
tylerkrupicka-stripe
pushed a commit
to stripe-archive/flow-to-typescript-codemod
that referenced
this pull request
Apr 11, 2025
* [Dep] upgrade to Recast from 0.20.4 to 0.23.4 **What?** Upgrade [recast](https://github.com/benjamn/recast) from `0.20.4` to `0.23.4` for more modern parsers. See [changes](benjamn/[email protected]). As part of the upgrade, the `[email protected]` patch is removed as it is no longer needed. **Why?** When the codemod with `[email protected]` removes comments, the parentheses can be incorrectly removed. Upgrading to `[email protected]` fixes this. For example, the following Flow type: ```js const a = // $FlowFixMe (1 + 1) * 2; ``` was incorrectly converted to: ```ts const a = 1 + 1 * 2; ``` With the new version, it gets correctly converted to: ```ts const a = (1 + 1) * 2; ``` Also, codemod with `[email protected]` keeps the following annotation as is: ```js type Props = { it: string, foo: number }; ```` With [this fix](benjamn/recast#1157) from v0.21.2, the Flow syntax gets correctly converted to the expected TypeScript syntax: ```ts type Props = { it: string; foo: number; }; ``` Lastly, v0.23.4 correctly handles unary expressions by [wrapping unary expressions in parens](benjamn/recast#1361) * Handle indexed access, `$Partial`, `$ReadOnlySet`, and `$ReadOnlyMap` **What** Add the following support (Flow syntax --> TypeScript syntax): - `T[K]` --> `T[K]` - `T?.[K]` --> `NonNullable<T>[K] | null | undefined` - `$Partial<T>` --> `Partial<T>` - `$ReadOnlySet<T>` --> `ReadonlySet<T>` - `$ReadOnlyMap<K, V>` --> `ReadonlyMap<K, V>` **Why** To support more Flow syntax. * Fix bugs with maybe function types and interaction types **Maybe function types** Flow code ```js ?() => void ``` was incorrectly transformed to: ```ts () => void | null | undefined ``` It is now correctly transformed to: ```ts (() => void) | null | undefined ``` **Intersection types** Flow code ```js (A | B) & (C | D) ``` was incorrectly transformed to: ```ts (A | B & C | D) ``` It is now correctly transformed to: ```ts ((A | B) & (C | D)) ``` * Fix false positives of privates types **What** Fix cases where `A$B` are public types instead of private types: 1. Relay generated types such as `PinRep_pin$data`` 2. Type Alias with prefix '$IMPORTED_TYPE$' **Why** To avoid false positives in the private types detection * Add override to force JSX through comment **What** If a file contain the comment `@jsx`, treat it as a JSX file. **Why** A mock file for a `.tsx` file must have the `.tsx` file extension for jest to find it. However, if the mock file does not contain any JSX, the codemod would use the file extension `.ts`. As a workaround, we introduce the ability to add a `@jsx` comment to the mock file to force the codemod to treat it as a JSX file. This ensures the mock file us es the `.tsx` extension, even if it doesn't contain any JSX. * Strip flow comments **What** - Remove flow-specific ESLint error suppression comments - Make sure to retain non-flow comments at the top of files **Why** The flow-related comments are no longer needed. Co-authored-by: Jack Hsu <[email protected]> * [react-router-dom] Improve handling of react-router-dom types **What** The following Flow code: ```js import { type Location, type Match, type RouterHistory } from 'react-router-dom'; type Props = { match: Match }; ``` gets correctly converted to TypeScript code: ```ts import { Location, History as RouterHistory } from 'history'; import { match } from 'react-router-dom'; type Props = { match: match<{ [key: string]: string | undefined }> }; ``` **Why** For the following types from `react-router-dom`: - The `Location` type is moved to `history`. - The `RouterHistory` type is moved to `history`, and is aliased to `History` instead. - The `Match` type becomes a generic type `match`. * [react] improve handling of React types 1. Handle named imports In addition to supporting `React.*` types: ```js import * as React from 'react'; type T = React.Node | React.Element | React.ChildrenArray; ``` We also support named imports: ```js import { type Node as ReactNode, type Element as ReactElement, type ChildrenArray } from 'react'; type T = ReactNode | ReactElement | ChildrenArray; ``` NOTE: To avoid name conflicts with the DOM `Node` and `Element` type, it is important to alias `React.Node` as `ReactNode` and `React.Element` as `ReactElement`. 2. Strip type annotations from React function component return types in favor of inference Flow: ```js const App = ({ message }: AppProps): React.Node => <div>{message}</div>; ``` TypeScript: ```ts const App = ({ message }: AppProps) => <div>{message}</div>; ``` Why not simply change `React.Node` to `React.ReactNode`? If we leave it as `React.ReactNode`, we'll receive the error `'Component' cannot be used as a JSX component`. To address this, we simply strip out the type annotation from the return type of the function component and allow TypeScript to infer it. 3. Strip out `AbstractComponent` in favor of inference Flow: ```js // @flow const C: React.AbstractComponent<Props, mixed> = React.forwardRef<Props, Ref>(Comp); export default (forwardRef<Props, Ref>(Comp): React.AbstractComponent<Props, mixed>); ``` TypeScript: ```ts const C = forwardRef<Ref, Props>(Comp); export default forwardRef<Ref, Props>(Comp); ``` Why? Because there is no `React.AbstractComponent` equivalent in TypeScript. We can simply strip it out and allow TypeScript to infer the type. 4. Reverse params for `forwardRef` Flow: ```js forwardRef<Props, Ref>(Comp); ``` TypeScript: ```ts forwardRef<Ref, Props>(Comp); ``` Why? Because the arguments are swapped in TypeScript. 5. Rename `ElementConfig`, `ElementProps`, `Portal`, and `StatelessFunctionalComponent` - `ElementConfig` --> `ComponentProps` - `ElementProps` --> `ComponentProps` - `Portal` --> `PortalProps` - `StatelessFunctionalComponent` --> `FC` --------- Co-authored-by: Mark Molinaro <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is very pretty similar to #868 but with
TsTypeLiteral. We were working on a codemod that involves adding to atypeand it was resulting in syntactically incorrect code like this (notice the;,):We were able to
patch-packagebut I figured I would make an upstream fix. Thank you!