Skip to content
This repository has been archived by the owner on Dec 5, 2024. It is now read-only.

more interfaces in typescript typings #153

Merged
merged 6 commits into from
May 1, 2018

Conversation

giladgray
Copy link
Contributor

@giladgray giladgray commented Apr 27, 2018

fixes #152

split typescript types into more interfaces that can be used separately in app code

import * as React from "react";
import { Manager, Popper, PopperChildrenProps, Reference, RefProps } from "react-popper";

export class Component extends React.Component {
    public render() {
        return <Manager><Popper>{this.renderPopper}</Popper></Manager>;
    }
    private renderPopper = (props: PopperChildrenProps) => {
        // profit! reuse the same function instance!
    }
}

split typescript types into more interfaces that can be used separately in app code
@FezVrasta
Copy link
Member

FezVrasta commented Apr 27, 2018

Thanks, is there any way to keep RefProps private? (not exported)

Also, I think it would make sense to export the ReferenceChildrenProps as well along with the PopperChildrenProps

}
export class Reference extends React.Component<ReferenceProps, {}> { }

interface ArrowProps extends RefProps {
Copy link
Member

Choose a reason for hiding this comment

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

PopperArrowProps instead? So we keep all the Popper related types under the same namespace?

FezVrasta pushed a commit that referenced this pull request Apr 27, 2018
These types may be useful to consumers to better type their own code.

This commit aims to get on pair with #153
@FezVrasta
Copy link
Member

I created this PR to mimic what you have done here.

I already renamed some types to reflect my comments in your PR, please let me know if it makes sense.

@giladgray
Copy link
Contributor Author

@FezVrasta not possible to prevent symbols in the .d.ts file from being exported, but i replaced RefProps with a simple RefHandler type.

interface PopperChildrenProps {
arrowProps: PopperArrowProps;
outOfBoundaries: boolean | null;
placement?: PopperJS.Placement;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@FezVrasta do we know if this field will always be defined? as in, should it be marked optional or not? my suspicion is that it's always defined, that popper.js always computes a valid placement regardless of the given option value

Copy link
Member

Choose a reason for hiding this comment

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

Yes actually I think it's always going to be defined, I'm not sure why I marked it as optional.

FezVrasta pushed a commit that referenced this pull request May 1, 2018
These types may be useful to consumers to better type their own code.

This commit aims to get on pair with #153
@FezVrasta FezVrasta merged commit 9b5f677 into floating-ui:master May 1, 2018
FezVrasta pushed a commit that referenced this pull request May 1, 2018
These types may be useful to consumers to better type their own code.

This commit aims to get on pair with #153
FezVrasta pushed a commit that referenced this pull request May 1, 2018
These types may be useful to consumers to better type their own code.

This commit aims to get on pair with #153
FezVrasta pushed a commit that referenced this pull request May 1, 2018
These types may be useful to consumers to better type their own code.

This commit aims to get on pair with #153
FezVrasta pushed a commit that referenced this pull request May 1, 2018
These types may be useful to consumers to better type their own code.

This commit aims to get on pair with #153
FezVrasta added a commit that referenced this pull request May 1, 2018
These types may be useful to consumers to better type their own code.

This commit aims to get on pair with #153
@giladgray giladgray deleted the patch-1 branch May 1, 2018 18:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

invalid syntax in 1.0 typings
2 participants