Skip to content

Обработка ошибок GraphQL#120

Merged
niyazm524 merged 12 commits into
devfrom
feature/error-handling
Nov 25, 2020
Merged

Обработка ошибок GraphQL#120
niyazm524 merged 12 commits into
devfrom
feature/error-handling

Conversation

@niyazm524

Copy link
Copy Markdown
Collaborator

Closes #4
Closes #31
localhost_3000_login

  • Компоненты для home page переиспользованы
  • Добавлены новые ошибки (блин)

@niyazm524 niyazm524 requested a review from azinit November 25, 2020 00:48
@niyazm524 niyazm524 self-assigned this Nov 25, 2020
@niyazm524

Copy link
Copy Markdown
Collaborator Author

#88 - на секунду показывается страница UserInfo

@github-actions

github-actions Bot commented Nov 25, 2020

Copy link
Copy Markdown

@azinit

azinit commented Nov 25, 2020

Copy link
Copy Markdown
Member

Хмм))

image

UPD: Чекни этот компонент, который для HomePage и для ошибок юзается

Чет с размерами тут

image

@azinit

azinit commented Nov 25, 2020

Copy link
Copy Markdown
Member

Пока некритично, но до показа бы исправить, что на какое-то время мелькает сама страница

Т.е. в идеале должна быть сначала проверка на доступ к ресурсу (для 404), и только если ок - уже отображаем сам page

(может имеет смысл какой-нить флаг accessLoading добавить на верхнем уровне, и только как она станет false - отображать чайлдов гарантированно)

В рамках PR - пофиг пока, но ишью бы выписать

#88 - на секунду показывается страница UserInfo

Да, да - поправить бы - выпиши issue себе

Comment thread src/app/error-handling/error-definitions.ts

@azinit azinit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Все круто, но го еще добьем моменты некоторые, чтоб точно до ума довести

Comment thread src/app/error-handling/error-catcher.tsx
Comment thread src/app/error-handling/error-catcher.tsx
Comment thread src/app/header/index.tsx
Comment thread src/app/header/index.tsx Outdated
Comment thread src/app/header/index.tsx Outdated
Comment thread src/app/utils.scss Outdated
Comment thread src/app/index.tsx Outdated
Comment thread src/features/home-hero/index.tsx
Comment thread src/pages/home/index.scss
Comment thread src/pages/home/index.tsx Outdated
@azinit azinit added this to the Спринт 2020_6 milestone Nov 25, 2020

@azinit azinit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Вливай. Остальное позже пофиксим

Молодец 👍

Реализация - пушка

Comment thread src/app/hocs/index.ts
import withRouter from "./with-router";

// Потом какой-нибудь `compose` метод заинсталлим откуда-нить и покрасивше будет
export const withHocs = (component: () => JSX.Element) => withRouter(withApollo(component));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Некритично, но в react-app-env уже задеклейрен глобально тип для компонентов

image

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

там с пропсами

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Так можно же props?: P выставить)

* Обертка для подключения и работы с API
*/
const withApollo = (component: () => React.ReactNode) => () => (
const withApollo = (component: () => JSX.Element) => () => (

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Некритично, но в react-app-env уже задеклейрен глобально тип для компонентов

image

Comment thread src/app/hocs/index.ts
import withApollo from "./with-apollo";
import withRouter from "./with-router";

// Потом какой-нибудь `compose` метод заинсталлим откуда-нить и покрасивше будет

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

😄

import { QueryParamProvider } from "use-query-params";

export const setupRouter = (component: () => JSX.Element) => () => (
const withRouter = (component: () => JSX.Element) => () => (

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Потом бы добавил коммент, аля

/**
 * Инициализация роутера с провайдером для работы с get-параметрами
 */

Comment thread src/app/index.tsx
import withApollo from "./with-apollo";
import Header from "./header";
import "./index.scss";
import { withHocs } from "./hocs";

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Круто, но стили в самом низу располагают обычно)

imports/order увы не удается пока для этого настроить

Comment thread src/app/index.tsx
import "./index.scss";
import { withHocs } from "./hocs";

const ErrorPage = lazy(() => import("pages/error"));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Чет смущает, что ErrorPage тут так хардкодится... Но ладно, потом посмотрим что с этим можно сделать. Потом в след коммитах пометь как-нибудь

// !!! FIXME: manage access

Comment thread src/app/header/index.tsx
q: currentTarget.value,
type: query.type,
s: query.s,
o: query.o,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Потом бы на такое заменил

const q = currentTarget.value;
history.push(`/search?${qs.stringify({ ...query, q })}`);

Comment thread src/features/home-hero/assets/github-icon-sad.svg
@niyazm524 niyazm524 merged commit cc9e19c into dev Nov 25, 2020
@niyazm524 niyazm524 deleted the feature/error-handling branch November 25, 2020 19:54
niyazm524 added a commit that referenced this pull request Nov 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment