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

Internal: TypeScript Migration #3567

Merged
merged 16 commits into from
May 16, 2024
Merged

Internal: TypeScript Migration #3567

merged 16 commits into from
May 16, 2024

Conversation

jackhsu978
Copy link
Member

@jackhsu978 jackhsu978 commented May 14, 2024

This PR migrates from Flow-typed JavaScript code to TypeScript code.

Breaking Change

We are dropping the Flow typing support for Gestalt!

Starting in Gestalt v148, Flow typings will not be exported. We will only export TypeScript typings going forward.

Q&A

Why migrate codebase to TypeScript?

Because we have decided to make TypeScript our standard language for all Pinterest web projects.

Why drop Flow support?

Maintaining both TypeScript and Flow typings requires significant amount of manual work. Since our active internal web clients consuming Gestalt have already been migrated from Flow to TypeScript, the additional work is not worth it.

Which directories are migrated?

  • /docs/
  • /packages/gestalt/src/
  • /packages/gestalt-charts/src/
  • /packages/gestalt-datepicker/src/

Why not migrate all directories?

While it's our goal to convert all Flow-typed JavaScript files to TypeScript files, it's low priority to migrate the internal code and potentially removable code (such as Gestalt upgrade codemod).

Why do we still have flow-typed directory?

This is because not all JavaScript code is migrated to TypeScript. Nonetheless, this PR removes most of the Flow library definitions.

Why change Element<typeof X> to ReactElement?

In TypeScript, it's not possible to limit a render prop to be a React element of a specific React component. See this doc

Note, that you cannot use TypeScript to describe that the children are a certain type of JSX elements, so you cannot use the type-system to describe a component which only accepts <li> children.

Why upgrade date-fns / @mui/x-date-picker?

  1. We are using date-fns v2 in some areas and v3 in some other areas. Upgrading makes it consistent.
  2. TS Typings is wrong in v2.16.1 and is only fixed in v2.17 Fix for package.json file generator (and ESM tree-shaking issues) date-fns/date-fns#2339
  3. Upgrading date-fns requires @mui/x-date-picker to also be upgraded. See [pickers] Error loading AdapterDateFns with date-fns v3 in vite: Missing "./_lib/format/longFormatters" specifier in "date-fns" package mui/mui-x#12144

Why add acornInjectPlugins to rollup config?

See https://www.npmjs.com/package/@rollup/plugin-typescript#preserving-jsx-output

Why docs/tsconfig.json and docs/next-env.d.ts?

The two files are actually auto-generated when I run yarn start locally. These files should not be manually edited.

Why check-in tsMigrate.sh?

The file isn't actually necessary and can be removed. I am just leaving it in for future reference.

@jackhsu978 jackhsu978 added the major release Major release label May 14, 2024
@jackhsu978 jackhsu978 requested a review from a team as a code owner May 14, 2024 19:54
@jackhsu978 jackhsu978 changed the title TypeScript Migration Internal: TypeScript Migration May 14, 2024
Copy link

netlify bot commented May 15, 2024

Deploy Preview for gestalt ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit c93baff
🔍 Latest deploy log https://app.netlify.com/sites/gestalt/deploys/66468b6a1da23b0008fc1804
😎 Deploy Preview https://deploy-preview-3567--gestalt.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@jackhsu978 jackhsu978 changed the title Internal: TypeScript Migration Breaking Change: TypeScript Migration May 15, 2024
@jackhsu978 jackhsu978 changed the title Breaking Change: TypeScript Migration Internal: TypeScript Migration May 15, 2024
@jackhsu978 jackhsu978 enabled auto-merge (squash) May 16, 2024 23:16
@AlbertCarreras AlbertCarreras self-requested a review May 16, 2024 23:16
Copy link
Contributor

@AlbertCarreras AlbertCarreras left a comment

Choose a reason for hiding this comment

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

CANT WAIT

@jackhsu978 jackhsu978 merged commit a3695d1 into pinterest:master May 16, 2024
19 checks passed
@jackhsu978 jackhsu978 deleted the ts branch May 21, 2024 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major release Major release
Projects
None yet
2 participants