-
Notifications
You must be signed in to change notification settings - Fork 2
Create initial react scaffold #1
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.
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
Conversation
| // eslint-disable-next-line @typescript-eslint/no-non-null-assertion | ||
| createRoot(document.getElementById("root")!).render( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this is a Vite thing or a React thing, but typescript-eslint doesn't like the non-null-assertion here. For this case I think it is fine to have, and removing it is more work than it's worth, so I think it's fine to ignore the linter here.
This rule may only be on because I turned strict checks on for tseslint, but we can always turn those off if it becomes a problem down the line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
ashstewart7
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, excited to be getting started on this!
| // eslint-disable-next-line @typescript-eslint/no-non-null-assertion | ||
| createRoot(document.getElementById("root")!).render( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
package.json
Outdated
| "preview": "vite preview" | ||
| }, | ||
| "dependencies": { | ||
| "react": "^18.3.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
React 19 was released a few weeks ago, let's upgrade to use that version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this needed to be a hard blocker on the PR, but I agree we should use the newest version of React.
Looks like the npm create vite@latest command hasn't been updated to use React 19 yet. I'll make the change.
Closes #2
Included is the initial scaffold for the site: