[#1936] Migrate c-authorship.vue to TypeScript

[vvidday]

Part of #1936

Proposed commit message

Currently, despite the addition of TypeScript support, the frontend is
still largely written in JavaScript. This results in a lack of type
safety and many complex objects being passed around as unknown types,
which may in turn lead to errors.

Let's migrate one of the main components, c-authorship.vue, to
TypeScript. This will provide better type safety and reduce runtime
errors.

Other information

Although this migration is fairly large, there aren't any logic changes and the PR mostly contains addition of types & minor refactoring to support said types.

These are the notable changes:

Removal of hasCommits() method

This PR removed the hasCommits() method. This method is not used anywhere in the codebase. It was written 4 years ago, but somewhere along the line the call to the method was removed but the method remained. The reason I had to remove it is because it references a non-existent property (repo.commits.authorFinalContributionMap) - this property does not exist on the object.

Duplicate interface/class

Similar to the User interface and class that was discussed in #1965, there is a duplicate definition between the AuthorshipFileSegment interface in types.ts and the Segment class. Since the context is identical (duplicate definitions, class only contains attributes and there's no class methods etc), we can address this in a separate PR, as discussed in #1965.

fileSize attribute potentially redundant

The fileSize attribute appears to be redundant. In the current codebase, this is the only time it is referenced, and since file.fileSize does not exist*, its value is always undefined (can be verified by opening Vue devtools and scrolling through the files array). We can look to remove this, but since this does not concern TypeScript migration, it can be done in a separate PR. Update: The assignment of this attribute, along with its type definition, has been removed in this PR due to the above reason.

*It is not exported by the backend when generating the authorship.json report.

@types/minimatch

The PR installs the above library as a dev dependency as it provides official typings for the minimatch library that we use in c-authorship. See https://www.npmjs.com/package/@types/minimatch

[ckcherry23]

The c-authorship file migration looks excellent to me! I haven't really found any places where you missed out on adding the type.

However, we may want to consider whether we want to restrict the return types of our functions to ensure the correct type is returned in functions with multiple return statements for example. Though the compiler may be able to infer what type we are returning, it may not be the type we intended. Hence, this will help us to avoid developer errors though it would make the code more verbose.

[vvidday]

The c-authorship file migration looks excellent to me! I haven't really found any places where you missed out on adding the type.

However, we may want to consider whether we want to restrict the return types of our functions to ensure the correct type is returned in functions with multiple return statements for example. Though the compiler may be able to infer what type we are returning, it may not be the type we intended. Hence, this will help us to avoid developer errors though it would make the code more verbose.

Great point! I originally didn't include explicit return type annotations as TypeScript can infer types, but as you pointed out, this could potentially result in incorrect return types (due to developer error) being 'accepted'. I think among the functions, the ones that are most important to annotate are the functions that have a non-void return type.

I'll update the function return types for this file and future files, but for previous files we could potentially add it as a first-time issue for new contributors, as it is relatively straightforward but will also give good exposure to the codebase (as it requires going through all the methods in the component and figure out how and what data flows between the functions).

[HCY123902]

Just a grammar check. Will it be more suitable to use the plural form lines instead

[vvidday]

I agree with this, I'll change it to lines. It's also similar in c-zoom, should we change it in #1965 too?

[HCY123902]

yeah sure. Please go ahead to also change it there

[vvidday]

@ckcherry23 thanks!

[zhoukerrr]

@vvidday Will appreciate if you can answer the clarification I have. Thank you!

[zhoukerrr]

May I check why is file.charCount added to the condition check? If file.charCount is 0, it will not enter the if branch. This is a little different from the original logic.

[vvidday]

The check was added because in the following line we check file.charCount <= SINGLE_FILE_CHAR_COUNT_THRESHOLD, but charCount is an optional field.
Thanks for pointing this out, here I think we have to do an explicit file.charCount !== undefined check to still allow the code to enter the branch if file.charCount is 0 like you mentioned.

If we check file.charCount !== undefined it technically won't be changing any logic, since charCount is only set when !file.isIgnored && !file.isBinary., so it will always be true.

[zhoukerrr]

LGTM!

[github-actions]

The following links are for previewing this pull request:

• Dashboard Preview: https://dashboard-1969-pr-reposense-reposense.surge.sh
• Docs Preview: https://docs-1969-pr-reposense-reposense.surge.sh