nsc-events_sprint_12_issue_561 #562
Conversation
…ed the recommendations from the material ui docs
Bejarano03
requested review from
bcko,
bennettsf,
IsaacJrTypes,
keiffer213,
JesseCaddell,
larsswenson,
miqo1385 and
ParysUnangst
Bejarano03
requested review from
cshimm and
brinkbrink
and removed request for
a team
|
Hello, |
|
Great job working on the TypeScript config file! I ran all the unit tests after the changes, and everything works successfully. All six test suites, including creator.test.tsx, admin.test.tsx, signin.test.tsx, and signup.test.tsx, executed without any issues, confirming that the TypeScript settings are properly integrated. The tests took around 8.5 seconds in total. Here’s a useful resource: TypeScript Compiler Options Explained. 
|
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
|
|
I'm just wondering if you see the same errors when you run npm run dev in your side? |
made some changes with version lens
update node to 20.x
There was a problem hiding this comment.
I'm also encountering the same issue that @miqo1385 is seeing:
I did have a conflict that I needed to resolve first which was downgrading react and react-dom to 18.2 (accepting current change) which I believe you were using (correct me if wrong). Not sure if this is the issue at hand or not. You may need to update this branch with main to see if that resolves the issue since its been a while at it says at the bottom.
Otherwise try this:
you can try to remove the import statements and simply specify the actual path in the src attribute for the Image components used by next/image. (have to go through each import for that in each file and correct them)
example:
<Image src="/images/google_play.png" alt="Google Play" width={100} height={100} />
I think you have to omit the public/ part because Next.js doesn't recognize the public folder as a module.
On a side note: I was also encountering some css warnings here in these images for the css files:
don't know if those were just on my end or not but thought it was worth mentioning.
not sure I should approve this just yet until these issues are addressed or clarified because it could still just be a small issue on our ends.
There was a problem hiding this comment.
Everything seems to work properly with these changes, would you be willing to explain what these changes are doing?
My understanding is that for rules such as "noImplicitAny": true,, it will show an error if a type for a variable (such as a function parameter) isn't explicit in the code?
There was a problem hiding this comment.
Looks good overall, I tested your branch and only found a minor issue with your package-lock.json file involving syntax errors:
Im wondering if you see the same issues, if not its probably on my end then.
Regardless I think you should update this branch with main (that can probably fix the issue) and there's also an issue with the CI/CD with one of the checks not going through I believe @JesseCaddell has solved the issue so I think updating the branch with main could potentially solve the issue as well (correct me if I'm wrong), or I think you have to make another PR for the check to trigger. Once you update your branch with main ill approve this PR, thanks for the work. @Bejarano03
Please fill out this description with the following:
Resolves 561
Description of what this pull request is for.
This pull request updates the typescript config file to follow recommendations from material ui.
Screenshot: