pr05 Typescript Migration #5: Migrate client/common folder #3565

clairep94 · 2025-08-03T12:46:57Z

pr05 Typescript Migration 5: Migrate the client/commonfolder

Context:

Rebuild of https://github.com/processing/p5.js-web-editor/pull/3564/files for cleaning commits
Migrate as much of the client/common folder with the following steps:

git mv someComponent.jsx someComponent.tsx. If possible, commit without the no-verify flag, otherwise with no-verify.
Add unit test to secure current behaviour
Check for all instances of useage for the common component to determine props types
Add props types & unit test update
Refactor if beneficial -- did minimal refactoring on this one compared to previous PRs and focused on typescript
Add JSDocs

Changes:

Didn't migrate the storebook files as I wasn't able to run storybook locally to verify against silent regression

Notes:

I have verified that this pull request:

has no linting errors (npm run lint)
has no test errors (npm run test)
is from a uniquely-named feature branch and is up to date with the develop branch.
is descriptively named and links to an issue number, i.e. Fixes #123

…ript parser on ts files

…omponents, remove react/require-default-props rule

clairep94 · 2025-08-03T13:05:05Z

@raclim @khanniie Ready for review! Can be reviewed separately from the utils folder migration PR

khanniie · 2025-08-13T22:56:38Z

client/common/Button.tsx

 import styled from 'styled-components';
-import { Link } from 'react-router-dom';
-
+import { Link, LinkProps } from 'react-router-dom';
 import { remSize, prop } from '../theme';

 const kinds = {


think this could also be a typescript enum

khanniie · 2025-08-13T22:56:54Z

client/common/Button.tsx


 const displays = {
  block: 'block',
  inline: 'inline'
+} as const;


khanniie · 2025-08-13T22:57:07Z

client/common/Button.tsx

+const buttonTypes = {
+  button: 'button',
+  submit: 'submit'
+} as const;


khanniie · 2025-08-13T22:57:48Z

client/common/Button.tsx

+  display: Display;
+};
+
+type SharedButtonProps = {


can this be an interface instead? same for all other similar cases with the Props

https://google.github.io/styleguide/tsguide.html#prefer-interfaces

khanniie · 2025-08-13T23:11:50Z

client/common/Button.tsx

+  /**
+   * Specifying an href will use an <a> to link to the URL
+   */
+  href?: string | null;


feels like if href can be undefined then we should avoid also letting it be null?

khanniie · 2025-08-13T23:13:13Z

client/common/Button.tsx

+   * Allows for IconButton to pass `focusable="false"` as a prop for SVGs.
+   * See @types/react > interface SVGAttributes<T> extends AriaAttributes, DOMAttributes<T>
+   */
+  focusable?: boolean | 'true' | 'false';


can we have it just be the boolean type? if the parent that is using this component has the information stored as a string we should have it be the parents' responsibility to cast it to a boolean type before it's passed in

khanniie · 2025-08-13T23:16:03Z

client/common/Button.tsx

@@ -181,69 +249,7 @@ const Button = ({
  );
 };

-Button.defaultProps = {


it's so great that we can get rid of all of this extra code at the bottom of the React files now haha, thanks for doing this conversion!!

khanniie · 2025-08-13T23:16:41Z

client/common/ButtonOrLink.tsx

 */
+export type ButtonOrLinkProps = {


can this be an interface?

khanniie · 2025-08-13T23:21:02Z

client/common/IconButton.tsx

+  ButtonProps,
+  'iconBefore' | 'display' | 'focusable'
+> & {
+  icon?: ComponentType<{ 'aria-label'?: string }> | null;


same here maybe prefer to remove the | null and let the nullish cases all be undefined

khanniie · 2025-08-13T23:30:14Z

client/common/usePrevious.ts

+ * @param value - The current value to track.
+ * @returns The previous value before the current render, or undefined if none.
+ */
+export default function usePrevious(value: number): number | undefined {


is there any way we can avoid having it be typed to numbers specifically? Maybe this is a rare case where it makes sense to use any or unknown? that way the hook can be used for more general cases in the future

…ent attributes

clairep94 · 2025-08-25T13:29:09Z

Ready for re-review @khanniie @raclim

khanniie

looks great!! thanks so much for all the changes!

khanniie · 2025-08-25T17:41:48Z

client/common/Button.tsx

-  secondary: 'secondary'
-} as const;
+enum Kinds {
+  primary = 'primary',


i think enums should use constant case for the fields! https://google.github.io/styleguide/tsguide.html#naming-rules-by-identifier-type

also, i don't think you really need the strings fwiw, there may be a few rare cases where the value itself gets used for some javascript/api logic but if it's used primarily as a categorization method in typescript-only code, i think we can do something like this?

enum Kinds { PRIMARY, SECONDARY, }

same thing for the others!

khanniie · 2025-08-25T17:42:38Z

client/common/Button.tsx

+   * Allows for IconButton to pass `focusable="false"` as a prop for SVGs.
+   * See @types/react > interface SVGAttributes<T> extends AriaAttributes, DOMAttributes<T>
+   */
+  focusable?: boolean | 'true' | 'false';


khanniie · 2025-08-25T17:49:17Z

client/common/Button.tsx

-Button.displays = displays;
-
-export default Button;
+Button.kinds = Kinds;


feels like instead of doing this, we could just export Kinds and Displays? like

export enum Kinds

and then other files can do this:

import {Kinds as ButtonKinds} from Button

clairep94 added 21 commits August 3, 2025 10:41

.prettierrc: remove hardcoded babel as parser setting to allow typesc…

f1af0a6

…ript parser on ts files

SkipLink: lint correctly to add ; to end of type definition lines

8a0f302

RouterTab: unit test

1cb8f5b

RouterTab: update to tsx --no-verify

d376207

RouterTab: add typescript & install @types/react-router-dom

29e1e6d

Button: update to tsx --no-verify

1efe93d

Button.tsx: migrate to typescript, add unit test, add @types/styled-c…

6042638

…omponents, remove react/require-default-props rule

ButtonOrLink: update to tsx files --no-verify

c1883c5

ButtonOrLink: update to typescript

eb69138

IconButton: update to tsx --no-verify

5dd03d1

IconButton: migrate to typescript & add unit test

197fd67

usePrevious: update to ts --no-verify

cfb4b82

usePrevious: migrate to typescript & add unit test

d80961c

isMac: migrate to typescript and add test

24368cc

useKeyDownHandlers: update to ts --no-verify

e67e015

useKeyDownHandler: migrate to typescript and add unit test

1b5373c

useModalClose: update to ts --no-verify

b5eea95

useModalClose: update to typescript & add test

7ca1e6d

useSyncFormTranslation: update to ts --no-verify

7075ffb

useSyncFormTranslations: update to typescript and add unit test

6bc9369

rename test utility rerender to mountComponent

94ea59a

clairep94 added the pr05 Grant Projects pr05 Grant Projects label Aug 3, 2025

clairep94 marked this pull request as ready for review August 3, 2025 13:04

clairep94 mentioned this pull request Aug 3, 2025

pr05 Typescript Migration: Update client/common folder to use the barrel folder structure clairep94/p5.js-web-editor#7

Open

4 tasks

khanniie reviewed Aug 13, 2025

View reviewed changes

khanniie requested a review from raclim August 13, 2025 23:43

clairep94 and others added 3 commits August 14, 2025 22:15

Merge branch 'develop' into pr05/migrate_client_common_rebuild

1b72ea4

.eslintrc: update to fix no-shadow eslint error on enums

7ce4f54

usePrevious: make type unknown to make more generic

7188c1b

clairep94 and others added 21 commits August 15, 2025 23:37

RouterTab: update TabProps to interface

bee3211

ButtonOrLink: update to interface

6148c33

IconButton: remove null

3e8faea

Button: WIP update

4bd9443

SkipLink: update type to interface

7718e02

.eslintrc: cherry-pick ts overrids from utils pr

3428dac

useSyncFormTranslations: update to be named export

1a7cfaa

eslintrc: fix double config

3676534

usePrevious: update to be named export

366aa7e

useModalClose: update to named export

2d8fe06

useKeyDownHandler: update to named export

b1788d5

RouterTab: update to named export

bc82be5

ButtonOrLink: update to named export

7b67549

IconButton: update to interface and named export

379e563

Button: update to named export

9515291

Button: clean up type definition to interface and add basic html elem…

dc7665d

…ent attributes

lint

0ff9ac1

Merge branch 'develop' into pr05/migrate_client_common_rebuild

a09c8d2

merge with main

7e7c1ab

Button: update jsdoc

67425a1

SkipLink: Update to named export

001c9e4

SkipLink: update test setup to use testutils

e8ba7b8

khanniie approved these changes Aug 25, 2025

View reviewed changes

Uh oh!

pr05 Typescript Migration #5: Migrate client/common folder #3565

Are you sure you want to change the base?

pr05 Typescript Migration #5: Migrate client/common folder #3565

Uh oh!

Conversation

clairep94 commented Aug 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

pr05 Typescript Migration 5: Migrate the client/commonfolder

Context:

Changes:

Notes:

Uh oh!

clairep94 commented Aug 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clairep94 commented Aug 25, 2025

Uh oh!

khanniie left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

clairep94 commented Aug 3, 2025 •

edited

Loading