AO
const reloadTable$ = new Subject<void>();Size: a a a
2020 August 22
ОС
Alex Okrushko
ok, т.е. в конце концов нужен тригер для перезапроса, так?
Получается так.
AO
Олександр Савуловський
Получается так.
тут несколько путей дальше. Если взять самый короткой (от того, что уже написано), то можно оставить эту функцию, и после
periodService.addPeriod(...).pipe(...).subscribe(() => this.reloadTable$.next());AO
В самом конструкторе (или в ngOnInit - хотя если не читаешь
@Input(), смысла ngOnInit нет) выстроить цепочку реакции:this.reloadTrigger$.pipe(
startWith(), // запуск первого запроса при инициализации все равно нужен
switchMap(() => this.periodService.getPeriodList()),
...
takeUntil(this._destroy),
).subscribe(...)
AO
к самом
так же
subscribe блоку тоже много вопросов, потому что в нем присваиваются значения переменным. Почему бы не вынести все в async pipe?так же
this.loginService.dateClosedPeriod = нарушает бест практис. Это получается, что Компонент присваивает значение у сервиса на прямую!AO
так же нет errorHandling нигде :(
если http запрос свалился, то юзер останется смотреть на спиннер :(
если http запрос свалился, то юзер останется смотреть на спиннер :(
AO
я бы все равно вынес работу с данными таблицы в отдельный сервис - и предоставил бы самому компоненту
ViewModel в виде Обзервабл.AO
может что-то в виде
readonly vm$: Observable<{tableData: Data, status: LoadingStatus, error: string}>AO
но это я бы так сделал. Если это слишком далеко забегаем, то хотя бы почистить subscribe
AO
this.reloadTrigger$.pipe(
startWith(), // запуск первого запроса при инициализации все равно нужен
tap(() => {
this.loading = true
}),
switchMap(() => this.periodService.getPeriodList().pipe(
tap(() => {
// это не должно здесь жить, но хотя бы переделай в функцию
this.loginService.setClose(new Date(data.maxPeriod));
}),
// inner Observable pipe, so that it doesn't collapse the outer stream on error
map(data => {
const periodList = data.response;
// тут все подсчеты можно сделать - это будет pure function, вне класса компонента (можно в том же файле)
const periods = calculatePeriods(data);
return {
...periods, // тут все твои dateStartPeriod и т.д.
periodList
};
}),
// handles both complete (it's a one-time Observable) and error cases
finalize(() => this.loading = false),
// do not let the error go through
catchError(() => EMPTY),
),
takeUntil(this.destroy), // это уже в outer Observable pipe
).subscribe();
AO
тут ты уже увидишь, что вместо
subscribe, можно сделать this.vm$ = this.reloadTrigger$.pipe(...)ОС
Alex Okrushko
this.reloadTrigger$.pipe(
startWith(), // запуск первого запроса при инициализации все равно нужен
tap(() => {
this.loading = true
}),
switchMap(() => this.periodService.getPeriodList().pipe(
tap(() => {
// это не должно здесь жить, но хотя бы переделай в функцию
this.loginService.setClose(new Date(data.maxPeriod));
}),
// inner Observable pipe, so that it doesn't collapse the outer stream on error
map(data => {
const periodList = data.response;
// тут все подсчеты можно сделать - это будет pure function, вне класса компонента (можно в том же файле)
const periods = calculatePeriods(data);
return {
...periods, // тут все твои dateStartPeriod и т.д.
periodList
};
}),
// handles both complete (it's a one-time Observable) and error cases
finalize(() => this.loading = false),
// do not let the error go through
catchError(() => EMPTY),
),
takeUntil(this.destroy), // это уже в outer Observable pipe
).subscribe();
Спасибо!
ОС
Alex Okrushko
к самом
так же
subscribe блоку тоже много вопросов, потому что в нем присваиваются значения переменным. Почему бы не вынести все в async pipe?так же
this.loginService.dateClosedPeriod = нарушает бест практис. Это получается, что Компонент присваивает значение у сервиса на прямую!С компонента в сервис я передаю значение через set
"this.loginService.dateClosedPeriod"
В сервисе:
set dateClosePeriod(value: Date) {
this._dateClosePeriod = value;
}
Это тоже не правильно?
"this.loginService.dateClosedPeriod"
В сервисе:
set dateClosePeriod(value: Date) {
this._dateClosePeriod = value;
}
Это тоже не правильно?
ОС
Alex Okrushko
this.reloadTrigger$.pipe(
startWith(), // запуск первого запроса при инициализации все равно нужен
tap(() => {
this.loading = true
}),
switchMap(() => this.periodService.getPeriodList().pipe(
tap(() => {
// это не должно здесь жить, но хотя бы переделай в функцию
this.loginService.setClose(new Date(data.maxPeriod));
}),
// inner Observable pipe, so that it doesn't collapse the outer stream on error
map(data => {
const periodList = data.response;
// тут все подсчеты можно сделать - это будет pure function, вне класса компонента (можно в том же файле)
const periods = calculatePeriods(data);
return {
...periods, // тут все твои dateStartPeriod и т.д.
periodList
};
}),
// handles both complete (it's a one-time Observable) and error cases
finalize(() => this.loading = false),
// do not let the error go through
catchError(() => EMPTY),
),
takeUntil(this.destroy), // это уже в outer Observable pipe
).subscribe();
Тут получается уже не нужно выносить в
async pipe. Правильно я понял?AO
Олександр Савуловський
С компонента в сервис я передаю значение через set
"this.loginService.dateClosedPeriod"
В сервисе:
set dateClosePeriod(value: Date) {
this._dateClosePeriod = value;
}
Это тоже не правильно?
"this.loginService.dateClosedPeriod"
В сервисе:
set dateClosePeriod(value: Date) {
this._dateClosePeriod = value;
}
Это тоже не правильно?
Это уже чуть лучше. Но проблема в том, что все, кому нужна инфа о dateClosePeriod, не узнаю об жто, пока сами не спросят (и прочитают данный). Я бы для начала внутри сервиса сделал её как BehaviorSubject
AO
Олександр Савуловський
Тут получается уже не нужно выносить в
async pipe. Правильно я понял?Следующий шаг был
vm$ - вместо takeUntil + subscribe() пустой, теперь можно просто в темплейт с async вынестиAO
Alex Okrushko
Это уже чуть лучше. Но проблема в том, что все, кому нужна инфа о dateClosePeriod, не узнаю об жто, пока сами не спросят (и прочитают данный). Я бы для начала внутри сервиса сделал её как BehaviorSubject
Не увидел сеттер там - нет, просто функцию используй
ОС
Alex Okrushko
Это уже чуть лучше. Но проблема в том, что все, кому нужна инфа о dateClosePeriod, не узнаю об жто, пока сами не спросят (и прочитают данный). Я бы для начала внутри сервиса сделал её как BehaviorSubject
А потом в сервисе делать bs$.next() полученных данных. Правильно?
ОС
Alex Okrushko
this.reloadTrigger$.pipe(
startWith(), // запуск первого запроса при инициализации все равно нужен
tap(() => {
this.loading = true
}),
switchMap(() => this.periodService.getPeriodList().pipe(
tap(() => {
// это не должно здесь жить, но хотя бы переделай в функцию
this.loginService.setClose(new Date(data.maxPeriod));
}),
// inner Observable pipe, so that it doesn't collapse the outer stream on error
map(data => {
const periodList = data.response;
// тут все подсчеты можно сделать - это будет pure function, вне класса компонента (можно в том же файле)
const periods = calculatePeriods(data);
return {
...periods, // тут все твои dateStartPeriod и т.д.
periodList
};
}),
// handles both complete (it's a one-time Observable) and error cases
finalize(() => this.loading = false),
// do not let the error go through
catchError(() => EMPTY),
),
takeUntil(this.destroy), // это уже в outer Observable pipe
).subscribe();
Я выношу весь код в конструктор. А потом при добавлении строки в таблицу нужно выполнить
periodService.addPeriod(...).pipe(...).subscribe(() => this.reloadTrigger$.next());. Правильно я понял?AO
Олександр Савуловський
Я выношу весь код в конструктор. А потом при добавлении строки в таблицу нужно выполнить
periodService.addPeriod(...).pipe(...).subscribe(() => this.reloadTrigger$.next());. Правильно я понял?Да.