Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
e3bfb66
add typescript support to issue-tracker
renanmav Nov 12, 2019
ec59071
revert prettier config
renanmav Nov 13, 2019
02ae738
replace non-null assertions for optional chaining on Root
renanmav Nov 13, 2019
a8424c6
throw if router doesnt exists on Link and remove unnecessary chaining…
renanmav Nov 13, 2019
f694d38
move RouteComponentProps to RouteRenderer
renanmav Nov 13, 2019
9abf9fb
avoid braceless
renanmav Nov 13, 2019
32c099d
update comment about @ts-ignore on RelayEnvironment.ts
renanmav Nov 13, 2019
bdb47e0
make Resource more precise
renanmav Nov 13, 2019
97007ce
revert IssueListItem to IssuesListItem
renanmav Nov 13, 2019
e2098e2
revert README changes
renanmav Nov 13, 2019
2c6a186
revert more README changes
renanmav Nov 13, 2019
d8411c5
revert e to event on IssueActions
renanmav Nov 13, 2019
0285f0f
force rename of GitHub
renanmav Nov 14, 2019
dd2786c
return Suspense with null fallback inside SuspenseList
renanmav Nov 14, 2019
4988da4
rename fragment IssueListItem to IssuesListItem
renanmav Nov 15, 2019
927e2eb
make result either T of obj with default
renanmav Nov 15, 2019
f6547ee
revert disable button on IssueActions
renanmav Nov 15, 2019
706cd71
revert the arrow function on Root
renanmav Nov 15, 2019
ab0b7ea
revert concat on RouteRenderer
renanmav Nov 15, 2019
58e89a3
make prepare a mandatory function
renanmav Nov 15, 2019
263a82e
use FetchFunction instead of typing function args
renanmav Nov 15, 2019
6807f0c
throw this.load() func on read() JSResource
renanmav Nov 15, 2019
da5ec35
remove optional chaining
renanmav Nov 15, 2019
a0d4660
throw if there isnt a router on RouteRenderer
renanmav Nov 18, 2019
d621941
remove unnecessary assertions on router
renanmav Nov 18, 2019
871bdca
add type guardian on JSResource
renanmav Nov 18, 2019
6836fbd
make RouteConfig entry general
renanmav Nov 19, 2019
af5b394
revert changes on README
renanmav Nov 19, 2019
7ddec0c
revert changes on README
renanmav Nov 19, 2019
01baff6
use React.FC
renanmav Nov 19, 2019
79e1a5e
add macro config to avoid false positive errors on yarn start
renanmav Nov 19, 2019
aa5def7
make all react components as React.FC
renanmav Nov 20, 2019
7cd9538
Merge branch 'master' into feat/issue-tracker-ts
renanmav Nov 20, 2019
cc1c367
Update README.md
renanmav Nov 20, 2019
5ba1807
Update .prettierrc
renanmav Nov 20, 2019
71ce2bf
remove unnecessary children prop
renanmav Nov 20, 2019
3d8ba57
Merge branch 'feat/issue-tracker-ts' of github.com:renanmav/relay-exa…
renanmav Nov 20, 2019
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
Prev Previous commit
Next Next commit
make all react components as React.FC
  • Loading branch information
renanmav committed Nov 20, 2019
commit aa5def74063863cdf30a0ccc5b5b5556b5e57acc
6 changes: 4 additions & 2 deletions issue-tracker/src/IssueActions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ interface Props {
issue: IssueActions_issue$key;
}

export default function IssueActions(props: Props) {
const IssueActions: React.FC<Props> = props => {
// Track the current comment text - this is used as the value of the comment textarea
const [commentText, setCommentText] = useState('');

Expand Down Expand Up @@ -178,4 +178,6 @@ export default function IssueActions(props: Props) {
</button>
</form>
);
}
};

export default IssueActions;
6 changes: 4 additions & 2 deletions issue-tracker/src/IssueDetailComments.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const SUSPENSE_CONFIG = { timeoutMs: 2000 };
/**
* Renders a list of comments for a given issue.
*/
export default function IssueDetailComments(props: Props) {
const IssueDetailComments: React.FC<Props> = props => {
// Given a reference to an issue in props.issue, defines *what*
// data the component needs about that repository. In this case we fetch
// the list of comments starting at a given cursor (initially null to start
Expand Down Expand Up @@ -119,4 +119,6 @@ export default function IssueDetailComments(props: Props) {
) : null}
</>
);
}
};

export default IssueDetailComments;
8 changes: 5 additions & 3 deletions issue-tracker/src/IssueDetailRoot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ interface Props {
/**
* The root component for the issue detail route.
*/
export default function IssueDetailRoot(props: Props) {
const IssueDetailRoot: React.FC<Props> = props => {
// Defines *what* data the component needs via a query. The responsibility of
// actually fetching this data belongs to the route definition: it calls
// preloadQuery() with the query and variables, and the result is passed
Expand Down Expand Up @@ -46,7 +46,7 @@ export default function IssueDetailRoot(props: Props) {
props.prepared.issueDetailQuery,
);
if (issue == null) {
return 'Issue not found';
return <div>Issue not found</div>;
}

return (
Expand Down Expand Up @@ -76,4 +76,6 @@ export default function IssueDetailRoot(props: Props) {
<IssueActions issue={issue} />
</div>
);
}
};

export default IssueDetailRoot;
6 changes: 4 additions & 2 deletions issue-tracker/src/Issues.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ interface Props {
/**
* Renders a list of issues for a given repository.
*/
export default function Issues(props: Props) {
const Issues: React.FC<Props> = props => {
// Given a reference to a repository in props.repository, defines *what*
// data the component needs about that repository. In this case we fetch
// the list of issues starting at a given cursor (initially null to start
Expand Down Expand Up @@ -74,4 +74,6 @@ export default function Issues(props: Props) {
</button>
</div>
);
}
};

export default Issues;
6 changes: 4 additions & 2 deletions issue-tracker/src/IssuesListItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ interface Props {
/**
* Renders a single item (issue) in the issues list.
*/
export default function IssuesListItem(props: Props) {
const IssuesListItem: React.FC<Props> = props => {
// Given a reference to a specific issue - props.issue - define *what*
// data the component needs about the issue in order to render it.
// Note that Relay will only give the component access to the exact fields
Expand All @@ -32,4 +32,6 @@ export default function IssuesListItem(props: Props) {

// Describe how to render the data:
return <Link to={`/issue/${issue.id}`}>{issue.title}</Link>;
}
};

export default IssuesListItem;
6 changes: 4 additions & 2 deletions issue-tracker/src/Root.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ interface Props {
children: React.ReactChildren;
}

export default function Root(props: Props) {
const Root: React.FC<Props> = props => {
// Defines *what* data the component needs via a query. The responsibility of
// actually fetching this data belongs to the route definition: it calls
// preloadQuery() with the query and variables, and the result is passed
Expand Down Expand Up @@ -44,4 +44,6 @@ export default function Root(props: Props) {
</section>
</div>
);
}
};

export default Root;
6 changes: 4 additions & 2 deletions issue-tracker/src/SuspenseImage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ type Props = React.DetailedHTMLProps<
HTMLImageElement
>;

export default function SuspenseImage(props: Props) {
const SuspenseImage: React.FC<Props> = props => {
const { src, alt } = props;

if (src != null) {
Expand Down Expand Up @@ -34,4 +34,6 @@ export default function SuspenseImage(props: Props) {
}

return <img alt={alt} {...props} />;
}
};

export default SuspenseImage;
6 changes: 4 additions & 2 deletions issue-tracker/src/routing/Link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ interface Props {
* An alternative to react-router's Link component that works with
* our custom RoutingContext.
*/
export default function Link(props: Props) {
const Link: React.FC<Props> = props => {
Copy link

@sorenhoyer sorenhoyer Nov 20, 2019

Choose a reason for hiding this comment

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

Any reason for not destructuring the props eg. const Link: React.FC<Props> = ({ children, to }) => {}? Same with all other instaces of React.FC

Also children is already defined by the FC type, so no reason to define it again in Props interface.

Copy link
Author

Choose a reason for hiding this comment

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

These destructures are really necessary? I think that they add an unnecessary complexity.


no reason to define it again

Thanks!

Choose a reason for hiding this comment

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

Isn't destructing props in functional components the standard? How is this complexity? If you did that you could remove 8 unnecessary props. just in that component.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, but I would add 8 unnecessary new lines to destructure. I'd have 16 lines between destructuring and usage against 8 lines with actual strategy.

In fact, using props.<propName> enforces that this data comes as props.

Choose a reason for hiding this comment

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

So... I guess this is pretty basic stuff but...
Changing const Link: React.FC<Props> = props => {} to const Link: React.FC<Props> = ({ children, to }) => {} means that, in just that Link component, you can remove props. from 7 instances of props.to and 1 instance of props. from props.children... how is this adding complexity to you... or 16 new lines. If anything it reduces redundancy (or what am I not seeing here?)

Sorry, what you say makes no sense to me. Thought everyone was doing it since ES2015. Maybe read this.

const router = useContext(RoutingContext);

if (router == null) {
Expand Down Expand Up @@ -51,4 +51,6 @@ export default function Link(props: Props) {
{props.children}
</a>
);
}
};

export default Link;
6 changes: 4 additions & 2 deletions issue-tracker/src/routing/RouteRenderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import './RouteRenderer.css';

const SUSPENSE_CONFIG = { timeoutMs: 2000 };

export default function RouterRenderer() {
const RouterRenderer = () => {
// Access the router
const router = useContext(RoutingContext);

Expand Down Expand Up @@ -111,7 +111,9 @@ export default function RouterRenderer() {
</Suspense>
</ErrorBoundary>
);
}
};

export default RouterRenderer;

/**
* The `component` property from the route entry is a Resource, which may or may not be ready.
Expand Down