Skip to content

Proof of concept test migration -- client > utils > getConfig #2

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

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from

Conversation

clairep94
Copy link
Owner

@clairep94 clairep94 commented May 18, 2025

Proof of concept for starting to migrate the client folder

Context:

  • Experimentation with starting to migrate the client folder to typescript and to set up automatic typescript checking

  • Starts to address work required in Month 1 and some of the challenges described in my proposal:

    • Installing dependencies required for Typescript & any peer-dependency bumps
    • Preventing silent regression by adding unit tests
    • Avoiding assumptions about input/output types when migrating
    • Adding tooling to automate checking for typescript errors
  • Chose to migrate utils/getConfig as the instance of a node file for Month 1 as this file had an associated unit test and the useage throughout the repo was relatively straightforward

  • I have a second PR that starts to migrate client/common/ButtonOrLink here which is separated from this PR for clarity

Changes

  • Installed typescript and typescript related deps/types for babel, node, react
  • Set up TSConfig in the root, which points to a TSConfig in the client (server will be added later)
  • Update webpack, babel and eslint configs to be able to parse .ts and .tsx files
    • Tried to add @typescript-eslint/parser and @typescript-eslint but got blocked due to eslint version peer-dependency
  • Add a package.json script typecheck:client to check for typescript errors on client files
  • Updated the husky pre-commit to use this
  • Added a test:watch script for easier unit test-writing
  • Migrated and re-factored client/utils/getConfig & updated unit tests
    • Checked all instances of useage in the repo and addressed each of the use-cases (input & output expected types)
    • Updated the isTestEnvironment() function to prevent circularity
    • Updated getConfig() to take an options object param that includes indicating whether it should output nullish strings ('') and/or output a desired type, such as a number
      • See tests for details. I have put examples of use-cases in the test comments
  • Update each file that calls getConfig()

Screenshots:

  1. Looking for instances of getConfig() useage
Looking for instances of getConfig being called
2. Example of instance where the nullish value expected should be `''` instead of `undefined` --> in this file, it would result in `'undefined'` if this update to `getConfig()` was not made Example of expecting a nullish string
3. Example of instance where nullish value expectd should be `undefined` -- getConfig can handle this as default Example of expecting undefined

clairep94 pushed a commit that referenced this pull request Jul 31, 2025
…_utils_instance

pr05 Typescript #2: automatically resolve imports of ts files & migrate instance of client/utils file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant