coopsoc / website

https://coopsoc.com.au
7 stars 5 forks source link

Migrate to TypeScript #68

Closed PenTest-duck closed 5 months ago

PenTest-duck commented 6 months ago

Migrated all files from JS and JSX to TS and TSX.

This means that most components now have an associated interface for its props.

The /data/types.ts contains type definitions for most objects.

Note:

  1. Some types, especially merch-store related types, have been hard to infer and may potentially be incorrect.
  2. Noticed some weird/ugly logic and code, but code cleanup is not in scope of this PR
  3. Seems like /calendar doesn't work the way prod does (which opens a webcal link) 🤔
  4. constants/index.ts for future usage
scorpiontornado commented 6 months ago
  1. Seems like /calendar doesn't work the way prod does (which opens a webcal link) 🤔

/calendar is a CloudFlare redirect, will only work in prod not localhost

scorpiontornado commented 6 months ago

Also should add tsc to the CI/CD pipeline, + maybe should add an npm/next script for typechecking

scorpiontornado commented 6 months ago

Would like to get #69 merged first, make sure to pull once that's done. Shouldn't have any conflicts hopefully

scorpiontornado commented 6 months ago

Also, make sure to use descriptive commit namings

lhvy commented 5 months ago

Would like to get #69 merged first, make sure to pull once that's done. Shouldn't have any conflicts hopefully

So much for no conflicts...

scorpiontornado commented 5 months ago

Lint issues + would probably be better to make all warnings errors image

scorpiontornado commented 5 months ago

Oh also - I wonder if we could merge Clickable and ClickableEvent somehow? Surely Clickable is just a degenerate case where T and U are void?

scorpiontornado commented 5 months ago

Oh also - I wonder if we could merge Clickable and ClickableEvent somehow? Surely Clickable is just a degenerate case where T and U are void?

ChatGPT suggests this, take it with a grain of salt but looks fine to me:

export interface Clickable<T = void, U = void> {
  onClick: (event: T) => U;
}

https://chatgpt.com/share/9d12b3fa-10ba-4183-8d67-e3ad29d3c4ef

PenTest-duck commented 5 months ago

Lint issues + would probably be better to make all warnings errors image

I would disagree because we're gonna run into silly little errors with TS (and sometimes, even known errors that the devs of libraries failed to fix - see /pages/blog/[slug].tsx) which will become unmanageable. However, I have made all warnings to errors - but at the cost of disabling eslint on a few lines.

PenTest-duck commented 5 months ago

Oh also - I wonder if we could merge Clickable and ClickableEvent somehow? Surely Clickable is just a degenerate case where T and U are void?

ChatGPT suggests this, take it with a grain of salt but looks fine to me:

export interface Clickable<T = void, U = void> {
  onClick: (event: T) => U;
}

https://chatgpt.com/share/9d12b3fa-10ba-4183-8d67-e3ad29d3c4ef

Turns out can't do that as the param event still exists even when we don't want it.

scorpiontornado commented 5 months ago

Oh also - I wonder if we could merge Clickable and ClickableEvent somehow? Surely Clickable is just a degenerate case where T and U are void?

ChatGPT suggests this, take it with a grain of salt but looks fine to me:

export interface Clickable<T = void, U = void> {
  onClick: (event: T) => U;
}

https://chatgpt.com/share/9d12b3fa-10ba-4183-8d67-e3ad29d3c4ef

Turns out can't do that as the param event still exists even when we don't want it.

Alright no worries, we can look for a way later but not a big deal