Closed typeofweb closed 1 year ago
przerost formy nad treścią.
Mimo, że minęło sporo czasu, to review jest nadal aktualne. Jak poprawisz te błędy, które znalazłem, to chętnie zerknę ponownie.
@houdini22 Review nadal aktualne, a może nawet bardziej niż dawniej – szczególnie w obliczu Twoich postów na Facebooku o problemach ze znalezieniem pracy.
Appka
div
).Kod
PropTypes
– niektóre propsy są tam dodane, a niektóre nie: https://github.com/houdini22/another-react-boilerplate/blob/5e0b0abf2f2a90969579e9a9dd1872715bccc643/WWW/src/js/components/form/TextArea/TextArea.js#L12-L14redux-form
nie jest zalecana, a trzymanie stanu każdego formularza w globalny storze jest błędem. https://github.com/houdini22/another-react-boilerplate/blob/5e0b0abf2f2a90969579e9a9dd1872715bccc643/WWW/src/js/store/configure-store.js#L5new Promise(…)
, więc to raczej sygnał ostrzegawczy. Przykładowo, w funkcjilogin
wystarczyłoby daćreturn
przedhttp.post
i new Promise stanie się zbędny. https://github.com/houdini22/another-react-boilerplate/blob/5e0b0abf2f2a90969579e9a9dd1872715bccc643/WWW/src/js/reducers/auth.js#L26-L41constructor
do tak prostych rzeczy, jak ustawienie stanu jest zbędne. https://github.com/houdini22/another-react-boilerplate/blob/5e0b0abf2f2a90969579e9a9dd1872715bccc643/WWW/src/js/components/form/Checkbox/Checkbox.js#L16-L18constructor
to już w ogóle do usunięcia. https://github.com/houdini22/another-react-boilerplate/blob/5e0b0abf2f2a90969579e9a9dd1872715bccc643/WWW/src/js/components/ui/ButtonGroup/ButtonGroup.js#L11-L13{(e) => this.handleClick(e)}
to to samo co po prostu{this.handleClick}
. https://github.com/houdini22/another-react-boilerplate/blob/5e0b0abf2f2a90969579e9a9dd1872715bccc643/WWW/src/js/components/form/Checkbox/Checkbox.js#L52FormField
jest przekomplikowany, należałoby go podzielić. Sygnałem ostrzegawczym jest już użycielet
(let inputComponent
) – jeśli się tego pozbędziesz i poprawisz, to będzie lepiej. https://github.com/houdini22/another-react-boilerplate/blob/5e0b0abf2f2a90969579e9a9dd1872715bccc643/WWW/src/js/components/form/FormField/FormField.js#L31Container
trzymasz elementy w state – to błąd. https://github.com/houdini22/another-react-boilerplate/blob/5e0b0abf2f2a90969579e9a9dd1872715bccc643/WWW/src/js/components/ui/Popover/Popover.js#L15-L22componentWillUnmount
–document.removeEventListener
nie usunie listenera, bo tworzysz nową funkcję. https://github.com/houdini22/another-react-boilerplate/blob/5e0b0abf2f2a90969579e9a9dd1872715bccc643/WWW/src/js/components/ui/Popover/Popover.js#L28 https://github.com/houdini22/another-react-boilerplate/blob/5e0b0abf2f2a90969579e9a9dd1872715bccc643/WWW/src/js/components/ui/Popover/Popover.js#L57AppContext.Provider
– przy każdym renderze tworzony jest nowy obiektvalue
a w nim nowe funkcje. https://github.com/houdini22/another-react-boilerplate/blob/5e0b0abf2f2a90969579e9a9dd1872715bccc643/WWW/src/js/components/ui/Popover/Popover.js#L109-L112this.handleOutsideHover = this.handleOutsideHover.bind(this)
jest zbędny, bo możesz używać arrow functions w klasach. https://github.com/houdini22/another-react-boilerplate/blob/5e0b0abf2f2a90969579e9a9dd1872715bccc643/WWW/src/js/components/ui/Popover/Popover.js#L177this.__reactstandin__isMounted
– wydaje się, że nigdy tego nie ustawiasz, zresztą jest to zbędne. https://github.com/houdini22/another-react-boilerplate/blob/5e0b0abf2f2a90969579e9a9dd1872715bccc643/WWW/src/js/components/ui/Popover/Popover.js#L189<AppContext.Consumer>
masz taki kod:this.contentElement = contentElement
– po co to przypisanie? Do jakiegothis
tutaj przypisujesz? https://github.com/houdini22/another-react-boilerplate/blob/5e0b0abf2f2a90969579e9a9dd1872715bccc643/WWW/src/js/components/ui/Popover/Popover.js#L221-L226Pewnie jest coś więcej, więc jak poprawisz te uwagi, to mogę zerknąć jeszcze raz :)
Pozdro!