-
-
Notifications
You must be signed in to change notification settings - Fork 445
Update dev dependencies #676
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
base: master
Are you sure you want to change the base?
Update dev dependencies #676
Conversation
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.
Small change regarding string coercion but looks good to me. @juliencrn will have to give final approval since they own it but I figure it doesn't hurt to have extra eyes on.
@@ -28,7 +28,7 @@ export default function Component() { | |||
return ( | |||
<> | |||
{Array.from({ length: 5 }).map((_, index) => ( | |||
<Section key={index + 1} title={`${index + 1}`} /> | |||
<Section key={index + 1} title={(index + 1).toFixed()} /> |
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.
Best to use .toString()
here. This would cause a difference in output. Template literals coerce their values to strings, which calls the .toString()
method instead of .toFixed()
let x = 123.456
x.toString() // "123.456"
x.toFixed() // "123"
<Section key={index + 1} title={(index + 1).toFixed()} /> | |
<Section key={index + 1} title={(index + 1).toString()} /> |
@@ -108,7 +108,9 @@ describe('useScrollLock()', () => { | |||
|
|||
const scrollbarWidth = window.innerWidth - document.body.scrollWidth | |||
|
|||
expect(document.body.style.paddingRight).toBe(`${scrollbarWidth}px`) | |||
expect(document.body.style.paddingRight).toBe( | |||
`${scrollbarWidth.toFixed()}px`, |
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.
Same as previous 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.
Should probably have marked for changes
🦋 Changeset detectedLatest commit: 7b74d4c The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Consolidate
typescript
dependency versions to a single one referenced in the rootpackage.json
. Before this change, they were two versions of TypeScript: 5.4.3 and 5.3.3. Now we have only one version of TypeScript: 5.4.3. To avoid running into warnings likewhen calling pnpm lint i had to update @typescript-eslint to 7.18.0. This required some fixes to add to the codebase.
I also added VS Code setting to prefer TypeScript version of workspace in IDE instead of the default one of VS Code itself.