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.

Sign up for GitHub

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

Jump to bottom

feat: replace JSX with TSX for type safety #35

Open

bartzalewski wants to merge 2 commits into Lunakepio:main from bartzalewski:feat/migrate-to-typescript

+2,475 −1,916

bartzalewski commented Feb 28, 2024

use as a starting point (?)


          feat: replace JSX with TSX for type safety

d0b2a73

vercel bot commented Feb 28, 2024 •

edited

Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
mario-kart-3-js	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 6, 2024 11:00pm

vercel bot deployed to Preview

February 28, 2024 23:57

View deployment

ShashankSuresh suggested changes

View reviewed changes

src/Landing.tsx Outdated

+                    }
+                  }
+                  const handleKeyDown = (event) => {

ShashankSuresh Mar 2, 2024

Suggested change

      
                const handleKeyDown = (event) => {
          
                const handleKeyDown = (event: KeyboardEvent) => {

src/Landing.tsx Outdated

+                const logo = useRef()
+                const startButton = useRef()
+                const homeRef = useRef()

ShashankSuresh Mar 2, 2024

Suggested change

      
              const homeRef = useRef()
          
              const homeRef = useRef<null | HTMLDivElement>(null)

src/Landing.tsx Outdated

+                const { gameStarted, actions } = useStore()
+                const logo = useRef()
+                const startButton = useRef()

ShashankSuresh Mar 2, 2024

Suggested change

      
              const startButton = useRef()
          
              const startButton = useRef<null | HTMLDivElement>(null)

src/Landing.tsx Outdated

+              export const Landing = () => {
+                const { gameStarted, actions } = useStore()
+                const logo = useRef()

ShashankSuresh Mar 2, 2024

Suggested change

      
              const logo = useRef()
          
              const logo = useRef<null | HTMLImageElement>(null)

src/components/models/characters/Mario_kart.tsx

		@@ -11,7 +11,13 @@ import FakeFlame from '../../ShaderMaterials/FakeFlame/FakeFlame'
		import { useStore } from '../../store'
		import gsap from 'gsap'

ShashankSuresh Mar 2, 2024

type MarioProps = {
  currentSpeed: number;
  steeringAngleWheels: number;
  isBoosting: boolean;
  shouldLaunch: boolean;
}

src/components/models/characters/Mario_kart.tsx Outdated

+                isBoosting,
+                shouldLaunch,
+                ...props
+              }) {

ShashankSuresh Mar 2, 2024

Suggested change

      
            }) {
          
            }: MarioProps) {

ShashankSuresh suggested changes

View reviewed changes

src/components/models/characters/Mario_kart.tsx

                 const [scale, setScale] = React.useState(1)
                 const { actions } = useStore()
                 const [shouldSlow, setShouldSlow] = React.useState(false)

ShashankSuresh Mar 2, 2024

Suggested change

      
              const [shouldSlow, setShouldSlow] = React.useState(false)
          
              const [,, setShouldSlow] = React.useState(false)

Author

bartzalewski Mar 6, 2024

Tuple type '[boolean, Dispatch<SetStateAction<boolean>>]' of length '2' has no element at index '2'.ts(2493)

Shouldn't that state be removed?

src/components/models/characters/Mario_kart.tsx Outdated

+                isBoosting,
+                shouldLaunch,
+                ...props
+              }) {
                 const { nodes, materials } = useGLTF('./models/characters/mariokarttest.glb')
                 const frontLeftWheel = useRef()

ShashankSuresh Mar 2, 2024

Suggested change

      
              const frontLeftWheel = useRef()
          
               const frontLeftWheel = useRef<Mesh>(null!)
          
              const frontRightWheel = useRef<Mesh>(null!)
          
              const rearWheels = useRef<Mesh>(null!)
          
              const frontWheels = useRef<Mesh>(null!)

src/components/models/characters/Mario_kart.tsx Outdated

                 const [scale, setScale] = React.useState(1)
                 const { actions } = useStore()
                 const [shouldSlow, setShouldSlow] = React.useState(false)
-                const mario = useRef();
+                const mario = useRef()

ShashankSuresh Mar 2, 2024

Suggested change

      
              const mario = useRef()
          
                const mario = useRef<Group<Object3DEventMap>>(null!);

ShashankSuresh suggested changes

View reviewed changes

ShashankSuresh left a comment

Hi - Good work in adding typescript.

We have a few areas where typescript is complaining. Could you please fix them?

ShashankSuresh commented Mar 2, 2024

For the file /workspaces/Mario-Kart-3.js/src/components/models/items/Banana_peel_mario_kart.tsx

please add props

type BananaProps = {
  onCollide?: number, 
  id: number, 
  position: number, 
  setNetworkBananas: SetStateAction<void>, 
  networkBananas: number
}

Author

bartzalewski commented Mar 6, 2024

Hi - Good work in adding typescript.

We have a few areas where typescript is complaining. Could you please fix them?

There are lots of areas where TypeScript is complaining. I think I would need some help here.


          fix: update types from ShashankSuresh

cd1655d

vercel bot deployed to Preview

March 6, 2024 23:00

View deployment

ShashankSuresh commented Mar 6, 2024

Hey. How can I help?
Can you let me know how I can contribute the same PR?

Author

bartzalewski commented Mar 7, 2024

Hey. How can I help? Can you let me know how I can contribute the same PR?

I believe every file in which there's a TypeScript error, must be fixed.

ShashankSuresh commented Mar 7, 2024

Hey. How can I help? Can you let me know how I can contribute the same PR?

I believe every file in which there's a TypeScript error, must be fixed.

Yeah. If you give me permission to fork this repo or raise a PR from the main repo, I can fix those type issues. First, we need to merge your PR. What do you suggest?

Author

bartzalewski commented Mar 7, 2024

Hey. How can I help? Can you let me know how I can contribute the same PR?

I believe every file in which there's a TypeScript error, must be fixed.

Yeah. If you give me permission to fork this repo or raise a PR from the main repo, I can fix those type issues. First, we need to merge your PR. What do you suggest?

Hmm, I didn't see any issues caused by my code, so I think it's ok to merge.

bartzalewski marked this pull request as ready for review

March 7, 2024 23:06

ShashankSuresh commented Mar 7, 2024

Hey. How can I help? Can you let me know how I can contribute the same PR?

I believe every file in which there's a TypeScript error, must be fixed.

Yeah. If you give me permission to fork this repo or raise a PR from the main repo, I can fix those type issues. First, we need to merge your PR. What do you suggest?

Hmm, I didn't see any issues caused by my code, so I think it's ok to merge.

I think so too. My only worry is if there is any linters during the build process. Anyway, please let me know once it's merged. I'll raise a new PR with the types fixed. 😀

Author

bartzalewski commented Mar 8, 2024

Hey. How can I help? Can you let me know how I can contribute the same PR?

I believe every file in which there's a TypeScript error, must be fixed.

Yeah. If you give me permission to fork this repo or raise a PR from the main repo, I can fix those type issues. First, we need to merge your PR. What do you suggest?

Hmm, I didn't see any issues caused by my code, so I think it's ok to merge.

I think so too. My only worry is if there is any linters during the build process. Anyway, please let me know once it's merged. I'll raise a new PR with the types fixed. 😀

ShashankSuresh commented Mar 12, 2024

Whats the latest update on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet