Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

v2.1 Implemented the Query Component #1398

Merged
merged 16 commits into from
Dec 22, 2017
Merged

v2.1 Implemented the Query Component #1398

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
c50a56a
Initial implementation of the Query component
excitement-engineer Dec 10, 2017
0e507a5
Added check for skip prop changes in willReceiveProps
excitement-engineer Dec 10, 2017
028f26e
Improved component, added more tests & created example illustrating u…
excitement-engineer Dec 16, 2017
23934e9
Added check to ensure that a Query document is passed in the <Query /…
excitement-engineer Dec 16, 2017
af7df0c
Added children render function.
excitement-engineer Dec 20, 2017
7d4ab66
Removed render prop in favor of children
excitement-engineer Dec 22, 2017
44706a5
Removed the loading and error render callbacks
excitement-engineer Dec 22, 2017
b1f6879
Removed the skip option
excitement-engineer Dec 22, 2017
e1e49be
Flattened the options to individual props
excitement-engineer Dec 22, 2017
42c84d3
Added fetchPolicy and notifyOnNetworkStatusChange to the props.
excitement-engineer Dec 22, 2017
a48b79d
Fixed minor issues
excitement-engineer Dec 22, 2017
9e682f5
Merge branch 'master' into Query
rosskevin Dec 22, 2017
faf3993
Fixed build errors
excitement-engineer Dec 22, 2017
581ffc2
Removed semicolon
excitement-engineer Dec 22, 2017
c94afd2
Fixed issue with semicolon conflict in ts
excitement-engineer Dec 22, 2017
9b2a5d2
Updated changelog
excitement-engineer Dec 22, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 4 additions & 17 deletions src/Query.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,13 @@ export interface QueryProps {
skip?: Boolean;
loading?: () => React.ReactNode;
error?: (error: ApolloError) => React.ReactNode;
render?: ((result: QueryRenderProp) => React.ReactNode);
children?: ((result: QueryRenderProp) => React.ReactNode) | React.ReactNode;
children?: (result: QueryRenderProp) => React.ReactNode;
}

export interface QueryState {
result: any;
}

const isEmptyChildren = children => React.Children.count(children) === 0;

function observableQueryFields(observable) {
const fields = pick(
observable,
Expand Down Expand Up @@ -180,7 +177,7 @@ class Query extends React.Component<QueryProps, QueryState> {
};

render() {
const { render, loading, error, children } = this.props;
const { loading, error, children } = this.props;
const result = this.getRenderProps();
Copy link
Contributor

Choose a reason for hiding this comment

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

I would just put getRenderProps implementation into render. Alternatively you could keep it and rename it to getResult to be consistent.


if (result.loading && loading) {
Expand All @@ -191,18 +188,8 @@ class Query extends React.Component<QueryProps, QueryState> {
return error(result.error);
}

if (render) {
return render(result);
}

if (typeof children === 'function') {
return children(result);
}

if (children && !isEmptyChildren(children))
return React.Children.only(children);

return null;
const renderedChildren = children(result);
return renderedChildren && React.Children.only(renderedChildren);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still have to have this limitation with React 16? We could allow more than one resulting child since now React allows it

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine with limiting to 16+ - but I'm on the progressive side. Not sure how other users would feel about it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd say yes, since this will be part of a new version, a new API. And even then, you can still use it in previous versions, only that it will fail with a less clear error when you attempt to render multiple children than the one React.Children.only gives you

}
}

Expand Down
Loading