-
Notifications
You must be signed in to change notification settings - Fork 2
Add TypeScript definitions #8
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
@@ -3,6 +3,7 @@ | |||
"version": "1.0.2", | |||
"description": "Respond to changes in a DOM element's size. With React Breakpoints, element queries are no longer \"web design's unicorn\" 🦄", | |||
"main": "dist/index.js", | |||
"types": "src/index.d.ts", |
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.
This file does currently not provide types for <Provider>
and useResizeObserver()
, because they are imported from another package (@envato/react-resize-observer-hook). I'm unsure whether re-exporting things from another library will pass through TS types, so I wasn't sure if I should add them in this PR, or if I should add them in the originating package only.
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 think you should provide types in that package. TS should be able to link things up? But i'm not 100% on this as i haven't met this usecase yet.
|
||
export const useResizeObserverEntry: (injectResizeObserverEntry?: ResizeObserverEntry) => ResizeObserverEntry | null; | ||
|
||
export const Context: React.Context<ResizeObserverEntry | null>; |
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.
When you use <Observe>
, it renders a React.Context
. Its value is the ResizeObserverEntry
. But because <Observe>
must first assign a React.RefObject
to the element-to-be-observed, the ResizeObserver can't provide a value instantly as there is nothing to observe yet. So this context initialises with null
.
|
||
export const useBreakpoints: <W extends any, H extends any>(options: BreakpointsOptions<W, H>) => [W, H]; | ||
|
||
export const useResizeObserverEntry: (injectResizeObserverEntry?: ResizeObserverEntry) => ResizeObserverEntry | null; |
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.
This hook accepts an optional ResizeObserverEntry
.
Internally, this hook accesses the context created by <Observe>
to retrieve its value, which is either a ResizeObserverEntry
or null
.
Then it returns the passed in argument, unless it's undefined
. In that case, it will return the context value, which can be either ResizeObserverEntry
or null
.
src/index.d.ts
Outdated
|
||
export const Observe: <E, W, H>(props: ObserveProps<E, W, H>) => JSX.Element; | ||
|
||
export const useBreakpoints: <W extends any, H extends any>(options: BreakpointsOptions<W, H>) => [W, H]; |
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.
This hook accepts an options object with this shape:
options = {
box: 'border-box' | 'content-box' | 'device-pixel-content-box',
widths: {
0: any,
769: any
},
heights: {
0: any,
1080: any
},
fragment: 0
};
All keys in options
are optional, but a user should provide at least either widths
or heights
. Not specifying both will cause useBreakpoints()
to always return [undefined, undefined]
. I don't know how I can communicate this soft constraint in TypeScript.
Inside the widths
and heights
objects, the keys are always numeric and should always increase. I don't know how to communicate this constraint in TypeScript. Specifying a lower key after specifying a higher key will cause the former to be missed by the hook's logic. I'm not re-ordering the widths
and heights
objects internally, nor do I think I should.
box
and fragment
are optional, and only work when your ResizeObserver
supports the draft spec box sizes and fragments.
box
can be one of undefined
, 'border-box'
, 'content-box'
, 'device-pixel-content-box'
.
fragment
can be undefined
or a 0-based positive integer.
This hook will return an array with two values.
const [widthMatch, heightMatch] = useBreakpoints(...)
Those values are the value belonging to the matching widths
key and heights
key respectively. The type of this value can be anything. I'm even allowing the value to be of a different type for each key in widths
and heights
. This was probably the hardest to write a type definition for, and your feedback would be greatly appreciated.
src/index.d.ts
Outdated
}: ObserveRenderArgs<E, W, H>) => JSX.Element; | ||
} | ||
|
||
export const Observe: <E, W, H>(props: ObserveProps<E, W, H>) => JSX.Element; |
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.
This component accepts the following props:
<Observe
box='border-box' // or `undefined`, 'content-box', 'device-pixel-content-box'
breakpoints={/* same interface as useBreakpoints */}
render={renderFunction}
/>
box
here is not the same as the one used in the options
object for the breakpoints
prop (or useBreakpoints()
, for that matter). It instructs the ResizeObserver which box of the element to watch. It has the same signature, though, so it can be undefined or any of the draft spec box sizes.
breakpoints
accepts an options
object identical to useBreakpoints()
. In fact, it's passed on to useBreakpoints()
in this component internally.
render
accepts a function which will be used to render your DOM children. It wraps them in a React.Context
and calls the function that you pass in. You'll get access to the following arguments:
const renderFunction = ({
observedElementProps,
widthMatch,
heightMatch
}) => { ... }
observedElementProps
is an object with the shape { ref: React.RefObject }
. You use it to apply ref
to a DOM element, and possibly other props in the future, like <div {...observedElementProps}>
.
widthMatch
and heightMatch
are only useful when you are using the breakpoints
prop. If you don't use that prop, these will always be undefined
. Otherwise, they will contain the values that useBreakpoints()
returns: [widthMatch, heightMatch]
.
@@ -0,0 +1,55 @@ | |||
import React from 'react'; | |||
|
|||
type BoxOptions = 'border-box' | 'content-box' | 'device-pixel-content-box'; |
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.
👍
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.
Seems legit - without knowing the inner workings of the application, the things that i'd expect to check (border-box, widths) looks good.
Have you tested with a :TS: project?
@doylem I have not. Would you consider updating to a pre-release to test this on your project? |
src/index.d.ts
Outdated
@@ -11,44 +11,44 @@ interface ResizeObserverEntry { | |||
readonly target: Element; | |||
readonly contentRect: DOMRectReadOnly; | |||
readonly borderBoxSize: Array<ResizeObserverBoxSize> | ResizeObserverBoxSize; | |||
readonly contentBoxSize: Arrray<ResizeObserverBoxSize> | ResizeObserverBoxSize; | |||
readonly devicePixelContentBoxSize: Arrray<ResizeObserverBoxSize> | ResizeObserverBoxSize; |
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.
🏴☠️
Disclaimer
I'm not fluent in TypeScript and I'm relying on the experience of my co-workers to check that these definitions are correct. To help with reviewing, for those who aren't sure what exactly this package does under the hood, I've added some comments below.
Context
This package is not written in TypeScript, and I'm currently not planning on rewriting it because I don't have enough experience in TS. However, many people do use TS and I want to make it easier for them to use this package.
Providing a definitions file seems like a nice middle ground.
Changes
Considerations
ResizeObserver
is implemented in Safari, ResizeObserver API declarations missing microsoft/TypeScript#37861 has started moving forward. Unsure what the exact impact will be on this package, but I assume I'll no longer need to manually specify interfaces forResizeObserverBoxSize
,ResizeObserverEntry
, etc.