Skip to content

Update dev dependencies #676

New issue

Have a question about this project? Sign up for a free account to open an issue and contact its maintainers and the community.

By clicking “Sign up for ”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on ? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

iwan-uschka
Copy link

Consolidate typescript dependency versions to a single one referenced in the root package.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 like

usehooks-ts:lint: =============
usehooks-ts:lint: 
usehooks-ts:lint: WARNING: You are currently running a version of TypeScript which is not officially supported by @typescript-eslint/typescript-estree.
usehooks-ts:lint: 
usehooks-ts:lint: You may find that it works just fine, or you may not.
usehooks-ts:lint: 
usehooks-ts:lint: SUPPORTED TYPESCRIPT VERSIONS: >=4.7.4 <5.4.0
usehooks-ts:lint: 
usehooks-ts:lint: YOUR TYPESCRIPT VERSION: 5.4.3
usehooks-ts:lint: 
usehooks-ts:lint: Please only submit bug reports when using the officially supported version.
usehooks-ts:lint: 
usehooks-ts:lint: =============

when 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.

Copy link

@lachieh lachieh left a 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()} />

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"
Suggested change
<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`,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as previous comment

Copy link

@lachieh lachieh left a 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-botchangeset-bot
Copy link

🦋 Changeset detected

Latest commit: 7b74d4c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
NameType
eslint-config-custom
usehooks-ts
www

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

Sign up for free to join this conversation on . Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants