Evgenii/lesson7#13
Conversation
|
|
||
| static getDerivedStateFromProps(props: ImageProps) { | ||
| return { | ||
| url: `https://picsum.photos/id/${props.id}/200`, |
There was a problem hiding this comment.
Шаблон было бы здорово вынести, встречатеся несколько раз по коду данная ссылка
|
|
||
| const getUrl = (id: number): string => { | ||
| const goodId = Math.min(1000, Math.max(1, id)); | ||
| return `https://picsum.photos/id/${goodId}/200`; |
There was a problem hiding this comment.
тогда шаблонную фукнцию создавать нужно - тут же просто разные примеры
There was a problem hiding this comment.
const getUrlWithID = (id: number) => `https://picsum.photos/id/${id}/200`| @@ -0,0 +1,66 @@ | |||
| import React from "react"; | |||
| import { ImageServer as Image } from "./ImageServer"; | |||
| // import { ImageLocal as Image} from "./ImageLocal"; | |||
| } | ||
|
|
||
| export class App extends React.Component<{}, ScreenState> { | ||
| constructor({}) { |
There was a problem hiding this comment.
constructor({}) { super({});
В данном случае подобное использование оправданно?
There was a problem hiding this comment.
Либо пустой объект же, либо null отправлять в super, не так ли? Когда у нас нет пропсов.
There was a problem hiding this comment.
constructor({}) { <- это деструктурация https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment (Unpacking fields from objects passed as function parameter)
ИМХО это может запутать, сперва идет диструктуразация, причем пустая и это нигде не используется.
Почему не так?
constructor(props) { super(props);
// or
constructor() { super({});
// or
constructor() { super(null));| render() { | ||
| const { id, interval, isActive } = this.state; | ||
| return ( | ||
| <div style={{ textAlign: "center" }}> |
|
|
||
| componentDidMount() { | ||
| getUrl(this.props.id).then((url) => { | ||
| this.setState({ url }); |
There was a problem hiding this comment.
вот я пропустил на занятии - было рассказано, что так можно получать ошибки, и что хорошо бы делать проверку? (я про случай, когда компонент удалили до резолва)
There was a problem hiding this comment.
И я про это забыл! Спасибо что напомнил. Сейчас напишу и нужно будет на следующем занятии об этом тоже сказать бы...
|
|
||
| componentDidUpdate(prevProps: ImageProps) { | ||
| if (prevProps.id !== this.props.id) { | ||
| getUrl(this.props.id).then((url) => { |
There was a problem hiding this comment.
ну и да, может это в метод вынести?
…revention of setting state on unmounted component, add emotion.
| @@ -0,0 +1,12 @@ | |||
| export const getUrl = (id: number): string => { | |||
| const goodId = Math.min(1000, Math.max(1, id)); | |||
There was a problem hiding this comment.
Как вариант, немного читабельней
Math.min([Math.abs(id), 1000]);
There was a problem hiding this comment.
проверка - что вернётся в случае id = 0?
0 - не верный id
There was a problem hiding this comment.
Я отталкивался от тесткейсов, такого теста в списке нету (
There was a problem hiding this comment.
Стоит исправить тесты в таком случае, я вижу 4 кейса: -1, 0, 1, 1001
@EJohnF Ничего не упустил?
| @@ -0,0 +1,12 @@ | |||
| export const getUrl = (id: number): string => { | |||
There was a problem hiding this comment.
Может стоит
export const ImageLink = "https://picsum.photos/id";
export const DefaultImageSize = 200;
type GetURLFunction = (id: number, size?: number) => string;
export const getUrl: GetURLFunction = (id, size = DefaultImageSize) => {
//...
return `${ImageLink}/${goodId}/${size}`;В этом случае можно очень красиво написать тесты, ИМХО
import { ImageLink, DefaultImageSize, getAsyncUrl, getUrl } from "./utils";
describe("Image utils", () => {
describe("Get local transformed links", () => {
it("test for correct id range 1-1000", () => {
const id = 1;
expect(getUrl(id)).toBe(`${ImageLink}/${id}/${DefaultImageSize}`);There was a problem hiding this comment.
И меняется ссылка в этом случае оч легко
…lesson7 # Conflicts: # README.md
3 participants
No description provided.