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

Migrate "components/app.jsx" and tests to Typescript #23439

Closed
mattermod opened this issue May 17, 2023 · 3 comments · Fixed by #23605
Closed

Migrate "components/app.jsx" and tests to Typescript #23439

mattermod opened this issue May 17, 2023 · 3 comments · Fixed by #23605
Labels
Area/Technical Debt Difficulty/1:Easy Easy ticket Good First Issue Suitable for first-time contributors Help Wanted Community help wanted Tech/TypeScript

Comments

@mattermod
Copy link
Contributor

We are commencing the migration of the mattermost-server/webapp to TypeScript in order to enhance code quality. This "Help Wanted" issue pertains to the migration of the aforementioned "JSX" files and their associated test files to TypeScript.

Below is a checklist outlining the tasks to be performed during the migration:

< ] Rename the files to their corresponding TypeScript extensions (e.g., js to ts, jsx to tsx).
[ ] Update any components that utilize these files with the correct imports.
[ ] Migrate the associated tests, if applicable.
[ > Address any errors raised by the TypeScript compiler. You can execute make check-types to view any encountered errors.

Our objective is to maintain a high level of stringency during this process. However, if you encounter any obstacles where successful migration would necessitate significant changes to other libraries or our tsconfig.json file, please raise the issues. We aim to ensure a smooth and gradual migration while enforcing as many changes as possible.

For reference, you can find examples of already migrated files in the following pull requests:
https://github.com/mattermost/mattermost-webapp/pull/3790
https://github.com/mattermost/mattermost-webapp/pull/3472
https://github.com/mattermost/mattermost-webapp/pull/4230


If you're interested please comment here and come join our "Contributors" community channel on our daily build server, where you can discuss questions with community members and the Mattermost core team. For technical advice or questions, please join our "Developers" community channel.

New contributors please see our Developer's Guide.

JIRA: https://mattermost.atlassian.net/browse/MM-52821

@tushar2708
Copy link

I would love to start working on this one.
I am new here, so have one question:
It seems that this change will spread across multiple files (mostly because of usage of changed variables), so will it be okay if the raised PR is a little big? Or would you expect something different?

@amyblais
Copy link
Member

I am new here, so have one question: It seems that this change will spread across multiple files (mostly because of usage of changed variables), so will it be okay if the raised PR is a little big? Or would you expect something different?

cc @M-ZubairAhmed

@M-ZubairAhmed
Copy link
Member

@tushar2708 thank you for coming forward to work on this.
Specific to this ticket I checked the usage of the above-mentioned file, And this is only imported in the entry file and is also used in the webapack config; so with that, the references to the file are only in two places. Please go ahead with the PR and we can continue the discussion there. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area/Technical Debt Difficulty/1:Easy Easy ticket Good First Issue Suitable for first-time contributors Help Wanted Community help wanted Tech/TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants