redwoodjs-com-archive icon indicating copy to clipboard operation
redwoodjs-com-archive copied to clipboard

Fix typescript errors

Open KrisCoulson opened this issue 2 years ago • 5 comments

yarn rw storybook fails to compile if there are typescript errors.

This is a WIP I fixed quite a few of the typescripts errors

I was having a little trouble running down why some were throwing.

@noire-munich would you be able to pick up from here and clean these last few typescript errors up. I also think there are a few tests that are failing at the moment from database schemas that have changed

KrisCoulson avatar Mar 04 '22 19:03 KrisCoulson

@realStandal @cannikin do you guys mind taking a look at this / merging? Lol I don't want it to get any bigger 🤣

KrisCoulson avatar Mar 06 '22 07:03 KrisCoulson

Sorry man, I hate Typescript with every fiber of my being and specifically didn't start the project as a TS one. I wouldn't know if the changes in there are correct or not! I'm also not worried about Storybook or tests at this point, I'm just trying to get everything live in time for the 1.0 launch. I figure we can circle back to those when we've got time. At least, that's my plan for job board—if others think they've got the time for the other sections of the site, that's great!

Should I just rename my files to .js and .jsx so I'm not creating new errors, or does that break everything? Once you enable TS for the project are you stuck with it everywhere, forever?

cannikin avatar Mar 06 '22 17:03 cannikin

@cannikin I actually figured out a way to turn off typescript in storybook in #10 so it won't fail to load if we try to spin it up. So no need to do anything differently.

I mainly just want to use storybook to test a few components and interactions out in isolation and think it would be good if it works.

And no worries. I get it. I haven't used much typescript myself. One thing I notice when making all these fixes that might make it a little less annoying is redwood generates the graphql types for you.

So you don't need to do this in your services

interface CreateJobProfileArgs {
  input: Prisma.JobProfileCreateInput
}

you can just

import { MutationcreateJobProfileArgs } from 'types/graphql'

it just prefixes Query or Mutation and then the name of your service

KrisCoulson avatar Mar 06 '22 18:03 KrisCoulson

TypeScript is... nuanced to say the least (I don't hate it - but I also think you don't need a sledgehammer to hang-up a picture frame). When @noire-munich and I first spoke, we agreed too that JavaScript would be easier/let us move faster. I think the generator is picking up on TypeScript now, so every new page, component, ... is being generated in TypeScript.

@cannikin From my understanding, and for the purpose of argument/in the context of RW, I think JavaScript and TypeScript can co-exist. There may be some issues when importing JS components/files into TS - but that'll be the type server breaking, not it being "wrong"; both sides are setup to support JS alongside TS.

All that to say, if your existing .ts and .tsx files aren't causing the application to fail to build, then I don't see a reason to have to change them over (I'm not factoring in Storybook working/not working). I don't mind correcting any errors I come across. The PR @KrisCoulson just opened should get around them causing Storybook to break anyway.

Anyway, @KrisCoulson, I don't mind checking this PR if you'd like to still merge it. But imo, I think disabling type checking (#10) to get Storybook to work until post-launch is perfectly fine; I noticed it errored out but had the same sentiments as Rob about leaving it be. And then we can call the project "JavaScript" and turn a blind eye to its TypeScript parts until the time is there, or the decision is made to remove any TypeScript 🤷

realStandal avatar Mar 06 '22 20:03 realStandal

@realStandal I don't mind forgetting this PR unless we just want to merge it to get it out of the way. Most of these types will probably need to be fixed in the future anyway.

Basically, I just ran yarn rw tsc saw the errors, and fixed them so it would stop yelling and compile correctly.

Since then I figured out how to disable typescript checking in storybook we just need to merge #10 then we don't have to worry about types and can fix them later down the road.

KrisCoulson avatar Mar 06 '22 21:03 KrisCoulson

Closing this as it is incredibly outdated.

KrisCoulson avatar Oct 25 '22 19:10 KrisCoulson