[Hauleth]

#programowanie #codereview #javascript

Nie jestem jakimś JSowym uberwymiataczem, więc się zgłaszam z prośbą do takowych. Ma ktoś czas by przejrzeć moje wypociny i ocenić jakość? Oraz zasugerować ewentualne poprawki:

https://github.com/hauleth/vanilla-ujs

[Marmite]

@Hauleth: Widzę, że nikt się nie odezwał, więc jeśli chcesz i będe mieć chwilkę wieczorem to chętnie Ci to review zrobię.

[Hauleth]

@Marmite: Spoko, aktualnie walczę z testami, by również pod FF się dobrze wykonywały. IE jeszcze nie testowałem (9+, niżej uważam, że nie ma sensu robić by też działało).

[Marmite]

@Hauleth: Podział plików wydaje mi się dosyć dziwny. Właściwie nie wiem w jakim celu jest (tzn zazwyczaj do osobnego pliku wydziela się osobny moduł, a nie tak jak tutaj dodanie event listenerów), przeglądam sobie plik confirm.js i widzę jakieś odwołania do funkcji matches czy confirm i teraz muszę szukać właściwie gdzie one siedzą i co robią.

csrf.js, linijka 13

origin = a.href.split('/', 3).join('/');

taki trochę magic number ta trójka, nie sądzisz?

[Hauleth]

@Marmite:

confirm

to funkcja wbudowana w przeglądarkę, więc ciężko bym ją sam definiował.

Co do

match

to to jest czysty polyfill na

matchesSelector

, która jest cały czas przedrostkowana w każdej przeglądarce, a nadpisywanie

Element

to raczej zły pomysł.

Cała ta funkcja jest dość magiczna ;)

Co do podziału plików to jest on zrobiony ze względu na funkcjonalność. Co do dodawania event listenetrów to jest to zrobione, by umożliwić kompatybilność z

[Marmite]

confirm to funkcja wbudowana w przeglądarkę, więc ciężko bym ją sam definiował.

@Hauleth: O rany, TEN confirm! Zapomniałem na śmierć że takie cuś istnieje.

Co do match to to jest czysty polyfill na matchesSelector, która jest cały czas przedrostkowana w każdej przeglądarce, a nadpisywanie Element to raczej zły pomysł.

Ludzie dzielą się na zaciekłych przeciwników rozszerzania wbudowanych obiektów oraz na tych, którzy nie widzą w tym nic złego. Ja nie widzę

[Hauleth]

@Marmite: Rozszerzanie natywnych obiektów - spoko. Rozszerzanie obiektów hosta - zuo. Wytłumaczenie.

Tak ten confirm. Ja go nie definiuję, bo zostawiam jego nadpisywanie już użytkownikowi gdyby chciał customowe wiadomości.

[Marmite]

Rozszerzanie natywnych obiektów - spoko. Rozszerzanie obiektów hosta - zuo. Wytłumaczenie.

@Hauleth: Poczytam, thx.