Migration to typescript by kamranayesh · Pull Request #101 · open-source-uom/myuom

kamranayesh · 2024-04-14T13:32:42Z

Overview
This pull request introduces comprehensive TypeScript (TS) support by migrating the existing JavaScript (JS) codebase to TypeScript and transforming JSX files to TSX. This initiative enhances code quality, provides static typing, and improves maintainability and scalability of the project.

Changes Made

TypeScript Migration:

Refactored all JavaScript files (.js) to TypeScript (.ts).
Updated type definitions and added explicit typings where necessary.
Applied TypeScript strict mode to catch potential errors at compile-time.

JSX to TSX Conversion:

Converted all JSX files (.jsx) to TypeScript JSX (.tsx).
Ensured proper type annotations for React components and props.
Utilized TypeScript's JSX intrinsic elements for enhanced type checking.

Configuration Updates:

Updated project configurations (e.g., tsconfig.json) to enable TypeScript compiler options and enforce strict type checking rules.
Integrated TypeScript linting and ensured compliance with coding standards.

Next Steps

Testing and Validation:

Ensure comprehensive testing to validate the migration and resolve any potential issues.
Solicit feedback from the community and incorporate suggestions for further enhancements.

Continuous Improvement:

Continuously monitor TypeScript adoption metrics and address any lingering migration tasks.
Encourage adoption of TypeScript among contributors and provide necessary support and documentation.

This migration from JavaScript to TypeScript and JSX to TSX represents a significant milestone in enhancing the project's robustness, maintainability, and developer experience. The transition leverages TypeScript's advanced features to elevate the project's code quality and position it for sustainable growth and community engagement.

#gsoc_participant
Kamran Ayesh

…d some data to typescript

papsavas

Thanks for this, it will REALLY help us moving forward. Couple of notes

papsavas · 2024-04-14T16:09:00Z

+const locations: Location[] = [
  {
-    floor: i18n.t("mezzanine_floor"),
+    floor: String(i18n.t("mezzanine_floor")),


Same thing as as string. If the result is null, we would be assigning 'null' as String(null) -> 'null'
Resolve it with i18n.t(....) ?? undefined

papsavas · 2024-04-14T16:16:54Z

-          //this is needed to make sure it rerenders when the office selected in the floor changes
          key={marked_position_x + " " + marked_position_y}
-          src={showGroundFloorImg ? GROUND_FLOOR_IMG_URL : imageURL}
+          src={showGroundFloorImg ? GROUND_FLOOR_IMG_URL : imageURL||""}


its safer to imageURL ?? "". If imageURL returns null for some reason, the above will accept null. Funny js moment where if it was "" || null, "" would be accepted

papsavas · 2024-04-14T16:17:50Z

        {showGroundFloorImg ? (
          <Text pt={"0.125rem"} px="1rem">
-            {i18n.t("enter_elevator_prompt")} {floor.toLowerCase()}{" "}
+            {i18n.t("enter_elevator_prompt")} {floor&&floor.toLowerCase()}{" "}


you can just floor?.toLowerCase() otherwise this might print false

papsavas · 2024-04-14T16:22:28Z

            key={index}
            value={option.title}
-            onClick={() => handleMenuItemClick(option.title)}
+            onClick={() => handleMenuItemClick(option.title||"")}


since useMapData handles empty strings we're ok always calling the handler, but it would be wiser to use option.title ?? undefined if for some reason this becomes nullable

papsavas · 2024-04-14T16:25:28Z

    <VStack>
      {/* <input type="text" onChange={onTextChangeHandler} /> */}
-      <SecrCard data={secretary} key={secretary.tel} />
+      {secretary&&<SecrCard data={secretary} key={secretary?.tel||""} />}


Avoid this conditional rendering method.
Use

{secretary ? <SecrCard key={secretary.tel} //this is defined, no need for optional chaining (?.) data={secretary} /> : null}

papsavas · 2024-04-14T16:25:55Z

          fontFamily="Syne"
          type="text"
-          placeholder={i18n.t("searchProf")}
+          placeholder={i18n.t("searchProf")||""}


?? undefined

papsavas · 2024-04-14T16:30:39Z

            >
              <UnorderedList spacing={2}>
-                {Object.values(foodMenu[key]).map((dish, index) => (
+                {Object.values(foodMenu[key as FoodMenuKey]).map((dish, index) => (


not really important but you could raise typing to the outer (Object.keys(foodMenu) as Array<keyof FoodMenu>).map(...) having no need to cast later

GeorgeFkd · 2024-04-16T08:16:00Z

Can you add further documentation regarding its deployment in docs/deployment.md? Is it still just npm run build and the rest still the same? Also should typescript's version be explicitly declared?(My JS ecosystem knowledge is limited) @papsavas can you provide your opinion in on this?

papsavas · 2024-04-16T09:52:59Z

Also should typescript's version be explicitly declared?

it is declared on package.json but devs need to use the workspace version instead of their system installed. In VSCode you can change it in the bottom right

GeorgeFkd · 2024-04-16T14:11:55Z

Thank you @papsavas, the README will soon have a section for the setup of the developer environment, @kamranayesh along with the other suggested changes make sure to include the documentation for the deployment as mentioned above and we will soon review to merge it and deploy it.

kamranayesh · 2024-04-16T15:32:57Z

Thank you @papsavas for your comments i will resolve them all. I will surely add documentation and update readme as well.
Open for any other suggestions.
@GeorgeFkd @GeorgeApos @Tsalmas-Anastasios

kamranayesh · 2024-04-19T12:42:09Z

@GeorgeFkd I have updated pr with resolved comments and added deployment instructions in readme.

shivarm · 2024-04-19T12:54:10Z

@kamranayesh resolve the merge conflicts

shivarm · 2024-04-19T12:56:52Z

@kamranayesh resolve the merge conflicts

and rebase or merge with main branch

shivarm · 2024-04-28T17:41:45Z

@kamranayesh How is it working locally?

kamranayesh · 2024-04-29T20:06:45Z

@shivam-sharma7 Its working fine. I request other contributors to checkout and test it too.

shivarm

I am seeing some error while reviewing this

Setting page layout is looking weird

when i have clicked http://localhost:3000/firstyearinfo the freshmen card then its show me the following errors

@GeorgeFkd @GeorgeApos @Tsalmas-Anastasios What do you think?

GeorgeFkd · 2024-04-30T08:18:51Z

It seems to have to do with the markdown files being used there. The freshmen articles are just markdown files rendered into react components using a library. The imports there i remember were a bit weird. If you cant resolve it i will investigate it in a few days.

papsavas · 2024-04-30T20:36:18Z

Are we planning to integrate vite here or in a future PR?
Right now, build script doesn't do much. If a type error is present, it doesn't catch it.

We should also avoid tsc, /dist and manual management of TS compiling in general.

vite react-ts template should be handy for this

shivarm · 2024-05-01T03:40:58Z

@papsavas First of all we have to deal with markdown files then it would be next step because I am getting same error while migrating in vite

GeorgeFkd · 2024-05-01T06:10:31Z

@papsavas I think we could do it on a next PR bcs this one is a huge chamge itself already. Also as @shivam-sharma7 mentioned we should investigate how to make markdown work in Vite.

papsavas

the "implicit inference" review is not crucial right now. I would prioritize the missed as string one to speed up the merging process. Its a long PR already, we don't need to make it longer

papsavas · 2024-05-10T19:23:21Z

-let departmentLocations = {
+import { Location } from "../../../../../hooks/useMapData";
+import { DepartmentLocations } from "..";
+const locations: Location[] = [...second_floor,...third_floor, ...fourth_floor];


you do not need to explicitly type this, its already infered as Location[] since all floors are. In general, prefer implicit inference. Code flows better that way

GeorgeFkd · 2024-05-24T08:24:13Z

As there are some legacy peer dependencies it needs npm install --force to run.

This will be temporarily allowed in order to push through this PR.

kamranayesh · 2024-05-25T20:34:35Z

As there are some legacy peer dependencies it needs npm install --force to run.

This will be temporarily allowed in order to push through this PR.

i updated package.json please try again

naman9271 · 2025-03-12T11:33:40Z

Hi @GeorgeFkd @GeorgeApos @papsavas I am interested in migration of this codebase to TS ... is this PR complete the migration or should i open mine for the migration of the codebase
Thanks

kamranayesh added 5 commits April 8, 2024 02:35

initial typescript configurations and migration of maps folder

78f09e4

migrated contexts, hooks , freshmen components, theme, added types an…

086fbbe

…d some data to typescript

migrated hooks , professors data to typescript

cd67e05

migrating pages to typescript

a4b63d9

migration from javascript to typescript . this resolve open-source-uom#5

4fc9afd

kamranayesh mentioned this pull request Apr 14, 2024

Migrate to Typescript to scale developer teams #5

Open

papsavas suggested changes Apr 14, 2024

View reviewed changes

kamranayesh added 2 commits April 19, 2024 18:04

add deployement intructions and resolved pr comments

19ba7c8

fix Readme

03fdd7e

kamranayesh added 2 commits April 24, 2024 11:09

resolving typescript version

da68cef

resolving conflicts

bfb480c

shivarm reviewed Apr 30, 2024

View reviewed changes

GeorgeApos requested review from GeorgeApos, GeorgeFkd and Tsalmas-Anastasios May 3, 2024 19:46

GeorgeApos added enhancement New feature or request help wanted Extra attention is needed labels May 3, 2024

GeorgeApos added p.High Priority: High frontend This issue is related to frontend backend This issue is related to backend quality This issue is related to quality and removed backend This issue is related to backend labels May 3, 2024

papsavas mentioned this pull request May 4, 2024

typescript: path aliases #124

Open

kamranayesh added 2 commits May 9, 2024 16:04

fixing lint error

ff4370f

resolving conflicts

63cc362

papsavas reviewed May 10, 2024

View reviewed changes

fixed setting drawer , other minor fixes , resolved comments

d29aa33

papsavas reviewed May 13, 2024

View reviewed changes

Comment thread src/contexts/departmentContext.tsx Outdated

GeorgeFkd added 2 commits May 24, 2024 10:17

Fixing minor issue

1ab8445

Fixed react-markdown major version update issue

a3598a8

updated package.json

6d9ad1e

shivarm mentioned this pull request Jun 6, 2024

Fix: prettier work locally and integrate with node.js CI #146

Merged

Tsalmas-Anastasios closed this Apr 12, 2025

Conversation

kamranayesh commented Apr 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

papsavas left a comment

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!

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!

GeorgeFkd commented Apr 16, 2024

Uh oh!

papsavas commented Apr 16, 2024

Uh oh!

GeorgeFkd commented Apr 16, 2024

Uh oh!

kamranayesh commented Apr 16, 2024

Uh oh!

kamranayesh commented Apr 19, 2024

Uh oh!

shivarm commented Apr 19, 2024

Uh oh!

shivarm commented Apr 19, 2024

Uh oh!

shivarm commented Apr 28, 2024

Uh oh!

kamranayesh commented Apr 29, 2024

Uh oh!

shivarm left a comment

Choose a reason for hiding this comment

Uh oh!

GeorgeFkd commented Apr 30, 2024

Uh oh!

papsavas commented Apr 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shivarm commented May 1, 2024

Uh oh!

GeorgeFkd commented May 1, 2024

Uh oh!

papsavas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

GeorgeFkd commented May 24, 2024

Uh oh!

kamranayesh commented May 25, 2024

Uh oh!

naman9271 commented Mar 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

kamranayesh commented Apr 14, 2024 •

edited

Loading

papsavas commented Apr 30, 2024 •

edited

Loading