-
Notifications
You must be signed in to change notification settings - Fork 99
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Nick/lesson18 #37
Nick/lesson18 #37
Conversation
Add ts:check to project Update dependencies && fix audit problem
@@ -85,3 +85,54 @@ export function* strangeZeroOneSequence(length: number) { | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Так же добавил пару примеров для урока, смешалось немного, блин (
@@ -26,7 +26,7 @@ export class GameSettingsFormState extends React.Component< | |||
player2Symbol: "O", | |||
}; | |||
|
|||
handleSubmit = (ev: React.FormEvent) => { | |||
handleSubmit = (ev: FormEvent) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
вот тут не понял, а что не так то было?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
а, неиспользованная переменная - ясно
@@ -11,4 +13,5 @@ export interface GameSettingsFormResult { | |||
|
|||
export interface GameSettingsFormProps { | |||
onSubmit: (settings: GameSettingsFormResult) => void; | |||
ref?: MutableRefObject<any>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
на секунду - а зачем это нужно? вернее это вроде не нужно, потому что ref не пробрасывается в пропсы
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Когда не было свойства TS выдавал ошибку, пришлось добавить
new BinaryTree("b", new BinaryTree("c"), new BinaryTree("d")), | ||
new BinaryTree("e") | ||
); | ||
const array = [...tree]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
а зачем тут спред?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Генератор выводит данные в массив, который уже и проверяем.
Иначе jest будет сравнивать шаги генератора, а не его содержимое
private left?: BinaryTree; | ||
private right?: BinaryTree; | ||
|
||
constructor(value: string, left?: BinaryTree, right?: BinaryTree) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
constructor(private value: string, private left?: BinaryTree, private right?: BinaryTree) {
и тело конструктора не понадобится
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Без конструктора не заводиться (((
Property 'value' has no initializer and is not definitely assigned in the constructor.ts(2564)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Принял, понял, не туда посмотрел, спасибо за пояснения
if (this.left) { | ||
yield* this.left; | ||
} | ||
if (this.right) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
не else if ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Нужно чекнуть параметры отдельно, они заходят по очереди при иницилизации
"include": [ | ||
"src/**/*", | ||
"scripts/**/*" | ||
], | ||
"exclude": [ | ||
"node_modules" | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ох не люблю я эти параметры
@saitonakamura что скажешь?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Я это сделал для того, чтобы можно было запустить проверку, если есть другой вариант, готов изменить
"ts:check": "npx tsc -p tsconfig.json --noEmit"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
да нормальные параметры, до этого он например пытался babel.config.js компилировать
No description provided.