pr05 Typescript #3: Migrate client/utils folder by clairep94 · Pull Request #3553 · processing/p5.js-web-editor

clairep94 · 2025-07-26T22:21:05Z

pr05 Typescript Migration 3: Migrate the client/utils folder

IMPORTANT: Should be reviewed after #3540 for clarity

This branch is built off of the branch for pr05 Typescript #2: automatically resolve imports of ts files & migrate instance of client/utils file #3540
Relevant commits start at 5b1fd1e

Context:

Rebuild of Pr05/migrate client utils clairep94/p5.js-web-editor#5 --> forgot to do git mv for the first few files and directly renamed, so this looks like deleting and creating a new file for git, and erases the previous history.
Migrate as much of the client/utils folder with the following steps:

git mv someUtil.js someUtil.ts. If possible, commit without the no-verify flag, otherwise with no-verify.
I forgot to add no-verify to my commit message in some of these initial ones
Add unit test to secure current behaviour
Check for all instances of useage for the utility function to determine types & edgecases of types
Add types & handle edgecases & update test
Refactor if beneficial
Add JSDocs

I skipped the intellisense autogen files & the codemirror utility files due to unfamiliarity, but migrated most of the rest of the folder

Changes:

Please see annotations on files

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

…recated

clairep94 · 2025-08-14T20:42:18Z

client/modules/IDE/actions/project.js

+const S3_BUCKET_URL_BASE = getConfig('S3_BUCKET_URL_BASE', {
+  throwErrorIfNotFound: true
+});
+const S3_BUCKET = getConfig('S3_BUCKET', { throwErrorIfNotFound: true });


@khanniie what do you think of this update?

getConfig options param now has throwErrorIfNotFound, which throws and error if the value is not found (or is an empty string)
This allows the compiler to throw errors immediately when attempting a local build if these things are missing

I've done a search for all instances where getConfig is used and made a guess of what I think would be useful to fail at compile-time if the config variable doesn't exist, but this is totally a guess

tysm!! appreciate you making the change! doesn't really affect the implementation but are we sure it's the compiler throwing the error and not the console at runtime? either way i think it's good we're throwing an error instead of letting it fail silently

Hmm I just tried this manually by removing API_URL from my local env and it didn't seem to throw in my local build, so I'm going to remove this!

Will look into doing the thing you mentioned where we have another module that checks for 'required' env variables at compile or build time

clairep94 · 2025-08-14T20:45:32Z

client/modules/Preview/EmbedFrame.jsx

-  )}`;
+  previewScripts.src = `${
+    window.location.origin
+  }${getConfig('PREVIEW_SCRIPTS_URL', { nullishString: true })}`;


@khanniie I don't have this in my local env, which is auto gen from the env.example, so I assumed this was not fail-on-not-found worthy, what do you think?

hmmmm tbh I don't really know if it's necessary or not, maybe @raclim would know?

I think it's created in server/views/previewIndex.js for rendering the preview! I think this is currently okay as is for now

client/modules/User/components/CookieConsent.jsx

clairep94 · 2025-08-14T20:48:53Z

client/utils/getConfig.ts

+  warn: !isTestEnvironment,
+  nullishString: false,
+  throwErrorIfNotFound: false,
+  throwErrorInTestEnv: false


@khanniie
Without this, I wasn't able to test if getConfig throws errors when we set {throwErrorIfNotFound: true} because the env is 'test' when we run the test

What do you think, is there another way to set the env of the test run for a single test?

Another alternative would be to do {throwErrorIfNotFound: !isTestEnv} for whereever we're using getConfig, but I was thinking maybe that's not the cleanest to keep doing that when we use the function

not sure if i understood the question exactly right, but i think there are ways to set the env for tests! i think you can override the values in the module?

https://stackoverflow.com/questions/48042200/invalidate-node-cache-when-using-jest

^ not sure if this above works for your case but we can definitely look into mocks / stubs for env if that's what you were wondering

khanniie · 2025-08-18T21:12:18Z

client/components/SkipLink.tsx

@@ -3,8 +3,8 @@ import classNames from 'classnames';
 import { useTranslation } from 'react-i18next';

 type SkipLinkProps = {


this should be interface i think?

khanniie

it's looking really good!!! I had a few more tiny comments but overall thank you so much for your hard work!

khanniie · 2025-08-18T21:13:47Z

client/modules/IDE/actions/project.js

+const S3_BUCKET_URL_BASE = getConfig('S3_BUCKET_URL_BASE', {
+  throwErrorIfNotFound: true
+});
+const S3_BUCKET = getConfig('S3_BUCKET', { throwErrorIfNotFound: true });


tysm!! appreciate you making the change! doesn't really affect the implementation but are we sure it's the compiler throwing the error and not the console at runtime? either way i think it's good we're throwing an error instead of letting it fail silently

khanniie · 2025-08-18T21:18:37Z

client/modules/Preview/EmbedFrame.jsx

-  )}`;
+  previewScripts.src = `${
+    window.location.origin
+  }${getConfig('PREVIEW_SCRIPTS_URL', { nullishString: true })}`;


hmmmm tbh I don't really know if it's necessary or not, maybe @raclim would know?

client/modules/User/components/CookieConsent.jsx

khanniie · 2025-08-18T21:25:34Z

client/utils/dispatcher.ts

+ *   - type: 'START', 'STOP' etc
+ *   - payload: additional data for that message type
+ */
+export type Message = {


khanniie · 2025-08-18T21:26:47Z

client/utils/evaluateExpression.ts

@@ -0,0 +1,44 @@
+type EvalResult = {


khanniie · 2025-08-18T21:28:33Z

client/utils/getConfig.ts

+  throwErrorIfNotFound?: boolean;
+}
+
+const defaultGetConfigOptions: GetConfigOptions = {


can we make this constant case? https://google.github.io/styleguide/tsguide.html#identifiers-constants

khanniie · 2025-08-18T21:35:45Z

client/utils/getConfig.ts

+  warn: !isTestEnvironment,
+  nullishString: false,
+  throwErrorIfNotFound: false,
+  throwErrorInTestEnv: false


not sure if i understood the question exactly right, but i think there are ways to set the env for tests! i think you can override the values in the module?

https://stackoverflow.com/questions/48042200/invalidate-node-cache-when-using-jest

^ not sure if this above works for your case but we can definitely look into mocks / stubs for env if that's what you were wondering

client/modules/IDE/components/Header/Nav.jsx

clairep94 · 2025-08-25T13:05:43Z

@khanniie @raclim Ready for re-review!

khanniie

thank you so much for all the work!!

raclim

Thanks so much for your work on this @clairep94 and for your review @khanniie! I think this overall feels, good and will merge this in!

clairep94 added 30 commits July 26, 2025 16:06

apiClient: update extension to ts

a52d312

apiClient: add axios instance type

2480bd1

device: update extension to ts

87adc72

device: add test

9efdd25

eslint: remove prefer-default-export rule

462524c

device.ts: add edgecase test and refactor

8010af7

metaKey.js: update extention to ts

cb9ddc5

metaKey: update to use isMac() -- due to navigator.platform being dep…

f7282c2

…recated

useKeyDownHandler: refactor to use isMac()

411eb4a

language-utils.js: update ext to ts

e7eaa2c

language-utils.ts: add test

53962db

language-utils: add types, passes typecheck

3efc164

language-utils: refactor for clarity & disable eslint noplusplus rule

fa6d69f

formatDate: update ext to ts

75b35a3

formatDate.js: add test

b7fb7a0

formatDate.js: add types and refactor

2440d6f

consoleUtils: update extension to ts

0a9af47

consoleUtils: add tests and bare minimum type

5302f54

consoleUtils.ts: add jsdocs and return type

2638c32

dispatcher.js: update extention to ts --no-verify

723cac3

dispatcher: add unit test

e0cb6c7

dispatcher.ts: update with types

0aab549

dispatcher.ts: add jsdocs

60f8a6e

remove jsdocs on internal functions to retain git history?

04ca29a

evaluateExpression: update ext to ts --no-verify

e91000f

evaluateExpression: add unit test

0713173

evaluateExpression: add tests

16d84e1

reduxFormUtils: update ext to ts --no-verify

6957124

reduxFormUtils: add unit test, no-verify

97754db

reduxFormUtils: delete unused dom-onlyprops function

ed0c6f0

getConfig: cleanup new optional property names

919ab3b

clairep94 commented Aug 14, 2025

View reviewed changes

cleanup unused import

de0f4da

clairep94 commented Aug 14, 2025

View reviewed changes

client/modules/User/components/CookieConsent.jsx Show resolved Hide resolved

clairep94 commented Aug 14, 2025

View reviewed changes

clairep94 added 3 commits August 14, 2025 21:59

reduxFormUtils: address comments for double partial

482fb18

evaluateExpression: update to named export

640a0bb

language-utils: remove check for navigatory == type of undefined

8b3fee5

clairep94 force-pushed the pr05/migrate_client_utils_folder branch from 999c62a to 8b3fee5 Compare August 15, 2025 19:00

clairep94 and others added 4 commits August 15, 2025 20:31

move isTestEnvironemnt to its own module for better testability

6de4d6d

getConfig: wip update to remove superfluous fail in test env option

0ad638f

fix test

c832736

Merge branch 'develop' into pr05/migrate_client_utils_folder

9fe530a

khanniie reviewed Aug 18, 2025

View reviewed changes

khanniie previously approved these changes Aug 18, 2025

View reviewed changes

clairep94 added 2 commits August 19, 2025 22:15

SkipLink: update type to interface

e11c82a

SkipLink: Update to named export

ed2114c

clairep94 dismissed khanniie’s stale review via ed2114c August 19, 2025 21:42

clairep94 and others added 7 commits August 19, 2025 22:52

parseStringToType: handle when original input is already a bool

c98ad97

getConfig: update to constant case

1c2ce16

evaluateExpression: update to interface

9f80ead

remove throwErrorIfNotFound from getConfig

92f2060

remove throwErrorIfNotFound from getConfig - useages

bd998b3

dispatcher: update to interface

8fe7b3e

Merge branch 'develop' into pr05/migrate_client_utils_folder

ec8de0a

khanniie approved these changes Aug 25, 2025

View reviewed changes

raclim approved these changes Aug 26, 2025

View reviewed changes

raclim merged commit 266e9e1 into processing:develop Aug 26, 2025
1 check passed

		@@ -3,8 +3,8 @@ import classNames from 'classnames';
		import { useTranslation } from 'react-i18next';

		type SkipLinkProps = {

Uh oh!

Conversation

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

Uh oh!

pr05 Typescript Migration 3: Migrate the client/utils folder

Context:

Changes:

Notes:

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!

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!

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!

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!

Uh oh!

clairep94 commented Aug 25, 2025

Uh oh!

khanniie left a comment

Choose a reason for hiding this comment

Uh oh!

raclim left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

clairep94 commented Jul 26, 2025 •

edited

Loading