Skip to content

Add jsdoc type checking for all of react code#132

Open
alansartorio wants to merge 4 commits into
mainfrom
jsdoc-type-checking
Open

Add jsdoc type checking for all of react code#132
alansartorio wants to merge 4 commits into
mainfrom
jsdoc-type-checking

Conversation

@alansartorio

@alansartorio alansartorio commented Mar 18, 2026

Copy link
Copy Markdown
Collaborator

Add yarn lint command that runs tsc type checking.
I think this will be helpful to prevent errors when changing stuff.

Some of the stuff I did:

Make code easier for tsc to prove.
Introduce "unreachable" util function that throws an error if reached (it shouldn't).
@jim3692

jim3692 commented Mar 19, 2026

Copy link
Copy Markdown
Collaborator

Great work Alan, but what are the benefits of using JSDoc, instead of switching to Typescript?

@IceDBorn

Copy link
Copy Markdown
Owner

I agree with @jim3692, we should use Typescript.

@alansartorio

Copy link
Copy Markdown
Collaborator Author

I also prefer typescript, it's cleaner for me. I just chose jsdoc because we were already using js, and didn't know if you chose js over ts for a reason.

The only benefits are that we can use it in the background and injector without adding a transpilation step (I only edited react though).

I will do the change to typescript in this same PR.

@jim3692

jim3692 commented Mar 19, 2026

Copy link
Copy Markdown
Collaborator

There is no need to change everything to TS. The injected code, and other parts that don't have type checking yet, can stay in JS.

There is this option in tsconfig: https://www.typescriptlang.org/tsconfig/allowJs.html

Also, to answer your question, we opted for JS, because we were too lazy to write TS and we didn't see enough benefits on such a simple UI component to go that route.

@alansartorio

Copy link
Copy Markdown
Collaborator Author

Yeah, I was only gonna change the react part to ts. Are you okay with this change to TS then?

@IceDBorn

Copy link
Copy Markdown
Owner

@alansartorio Back then, our scope was not that big either 😛

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.

3 participants