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

[#1632] Frontend: Add TypeScript #1721

Merged
merged 15 commits into from
Mar 30, 2022
Merged

Conversation

zhoukerrr
Copy link
Contributor

@zhoukerrr zhoukerrr commented Mar 17, 2022

Fixes #1632

Proposed commit message

TypeScript offers precise definition through typing, which makes code
management easier.

Let's start transitioning our frontend code to use TypeScript.

Other information

Please refer to the comments below for more information.

@zhoukerrr
Copy link
Contributor Author

For record purposes, these are the steps taken to add typescript.

In /frontend folder:

  1. Remove comments from all .json files.
  2. Enter vue add typescript.
  3. Choose the following options
    Screenshot 2022-03-18 at 22 15 58
  4. Update the entry point in vue.config.js from main.js to main.ts.
  5. Downgrade @vue/cli-plugin-typescript to ~4.5.0 to match the other vue cli plugins.

@zhoukerrr
Copy link
Contributor Author

zhoukerrr commented Mar 23, 2022

Justification for the decision.

Class-style component syntax

In short, this will require us to change the syntax to export default class Counter extends Vue. This is not the style we use in the project now. The only advantage we get from this is that we can utilize some ECMAScript language features such as class inheritance and decorators, which is not used the project now. Changing the syntax now is only going to add additional work to the integration of typescript. For more details, please refer to here.

Babel

We are already using @vue/cli-plugin-babel. Therefore, it makes sense to continue this for typescript. Furthermore, this is a common pattern for projects with existing build infrastructure which may have been ported from a JavaScript codebase to TypeScript. This suits our current need as well. For more information, please refer to here. This contains some guide on fixing certain issues.

DO NOT convert all .js to .ts

It will be too much work to change everything together. Therefore, I think it is better to change parts by parts to ensure individual part work before we change everything to .ts.

Allow .js files to be compiled

We need this because most of the files are still in .js.

Skip type checking for all declaration files

Although it is not a default, typescript recommends this for apps. The details can be found here.

Downgrading @vue/cli-plugin-typescript to ~4.5.0

vue add typescript installs the latest version. This causes an issue with the existing vue cli plugins. We are unable to upgrade @vue/cli-service to a higher version because of the node version used in this project. We can consider upgrading it once we upgrade the node version.

@zhoukerrr zhoukerrr marked this pull request as ready for review March 23, 2022 03:54
@zhoukerrr zhoukerrr requested a review from a team March 23, 2022 03:55
@gerhean gerhean requested a review from a team March 24, 2022 06:04
},
"extends": [
"airbnb-base",
"plugin:vue/recommended",
Copy link
Contributor

@gerhean gerhean Mar 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really okay to uncomment this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also the other uncommented things, actually can you work on them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gerhean I did not uncomment those, I just removed them entirely. I think we can add it back now but since it is not in used, I think we can just remove those to avoid any potential future errors.

@gerhean
Copy link
Contributor

gerhean commented Mar 24, 2022

Can you rewrite main.ts in typescript as a way to showcase it? Since it is a relatively small file as well, so it should be easy to convert.

Copy link
Contributor

@gerhean gerhean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gerhean gerhean requested a review from a team March 25, 2022 02:01
@dcshzj dcshzj changed the title [#1632] Add typescript [#1632] Frontend: Add TypeScript Mar 30, 2022
@dcshzj dcshzj merged commit 2b86402 into reposense:master Mar 30, 2022
@github-actions
Copy link
Contributor

The following links are for previewing this pull request:

@vvidday vvidday mentioned this pull request Mar 11, 2023
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add TypeScript
4 participants