v2.0.0 - Typings issue for Breakpoint and useCurrentWidth/useCurrentBreakpointName

[dbryantm]

The new TypeScript typings break custom breakpoint names for Breakpoint (i.e. sm, md, lg, etc). I currently get the following error:

Type '{ children: Element; sm: true; down: true; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<Breakpoint> & Readonly<Props> & Readonly<{ children?: ReactNode; }>'.
  Property 'sm' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<Breakpoint> & Readonly<Props> & Readonly<{ children?: ReactNode; }>'.

Just a suggestion, but either have a breakpoint prop that accepts a string with the following typing (also probably more maintainable):

interface Props {
  children?: React.ReactNode;
  breakpoint?: string;
  up?: boolean;
  down?: boolean;
  only?: boolean;
  tagName?: string;
  className?: string;
}

Or change the Props typing to the following (which unfortunately lets more in but doesn't restrict the use of the library):

interface Props<T extends any> {
  children?: React.ReactNode;
  up?: boolean;
  down?: boolean;
  only?: boolean;
  tagName?: string;
  className?: string;
}

Also, you're missing typings for useCurrentWidth and useCurrentBreakpointName:

export function useCurrentWidth(): number;
export function userCurrentBreakpointName(): string;

[guschnwg]

My breakpoints are small, medium and large, somewhere in my app I added the file socks.d.ts with the following code:

namespace ReactSocks {
  interface Props {
    children?: React.ReactNode;
    up?: boolean;
    down?: boolean;
    only?: boolean;
    tagName?: string;
    className?: string;
    small?: boolean;
    medium?: boolean;
    large?: boolean;
  }
}

[dbryantm]

I've already done something very similar. Though it would probably be good to either document it on the README or update the typings to make them work as advertised. Also, your example doesn't account for the missing hook typings which should at the least be included in the type definitions. I like the library and it's also mainly a suggestion to prevent developers with less TypeScript experience running into issues.

[guschnwg]

Totally agreed, as my suggestion was only a temporary solution.

As my react app was not compiling with that error you mentioned, that temporary solution solved the issue for me, and could solve (tomporary) to another person who read your issue and was hurting only for the first part

[flexdinesh]

@dbryantm Thanks for the report. Would you be interested in improving this? Quite honestly TypeScript is a bit out of my depth.

[flexdinesh]

@nutboltu Perhaps you'd like to add something to this discussion?

[nutboltu]

@dbryantm and @flexdinesh It seems, we are expecting any props (see here) and replace it with breakpoint. In this case consumer can pass any props (sm, md, lg, small, medium, large ).

Possible Solution: 1

As @dbryantm mentioned, we can introduce a new props breakpoint where consumer can pass custom breakpoint.

<Breakpoint breakpoint="small" down>
</Breakpoint>

Possible Solution: 2 We can add an unknown property in the Props

interface Props {
      children?: React.ReactNode;
      up?: boolean;
      down?: boolean;
      only?: boolean;
      tagName?: string;
      className?: string;
      [breakpoint: string]: string
    }

Let me know your thoughts.

[flexdinesh]

@nutboltu When we started, the biggest highlight of this lib was its sugary syntax (being able to pass multiple breakpoints with shorthands) and I'd like to keep it that way unless there's a performance or demonstrated bad practice in play.

Solution 2 seems convincing to me. It will keep TS consumers happy while retaining the syntax. What do you all think? @dbryantm @guschnwg @nutboltu

[dbryantm]

@flexdinesh @nutboltu My radio silence was due to me trying to think of better options. I like the computed property option though I would keep inline with making it a boolean:

interface Props {
  children?: React.ReactNode;
  up?: boolean;
  down?: boolean;
  only?: boolean;
  tagName?: string;
  className?: string;
  [breakpoint: string]: boolean;
}

Let me know your thoughts. I could submit a PR for this and the two hooks that need to export their typings also.

[nutboltu]

+1 for Solution 2 then 🎉 .

[flexdinesh]

@dbryantm Sounds great. Please go ahead with PR. 🎉

[dbryantm]

Just an FYI, if you look at the following example:

TypeScript Playground - Proposed Solution Issues

I think we're going to have to go with something closer to my first suggested solution. I'll still go ahead and submit a PR but would like to get your thoughts using that as evidence for my findings. Thanks in advance!

[flexdinesh]

@nutboltu Would you be able to review the PR? TypeScript is a bit out of my depth.

[nutboltu]

@dbryantm Thanks for fixing the unknown props issue in a cleaner way. it looks good to me.

[flexdinesh]

Changes published in v2.0.1. Thanks @dbryantm @nutboltu @guschnwg! 🎉