Skip to content
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

Build system refactor + ES6 refactor + upgrade dependancies #122

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

kubik369
Copy link
Collaborator

@kubik369 kubik369 commented Mar 3, 2018

Vopred sa ospravedlňujem komukoľvek, kto toto bude čítať, ak sa vôbec niekto taký nájde, dúfam, že áno.

Tento PR je prakticky zložený z 2 commitov, 81093b3 (CSS) a e8755ce (JS). e33957b je len zmazanie všetkého starého JS sourcu, pretože git si nevšimol, že sa len pohol. Zvyšné sú drobnosti, ktoré nie sú také bolestivé. Prvé 2 commity (59cca25 a 0e27ceb) sú rozdiely, ktoré sa mi tak trochu nechcelo kopať aby som vyriešil.

CSS commit robí 2 veci. Bol v podstate kompletne zahodený buildstatic.sh a bol prerobený na buildcss.sh. CSS je teraz rozdelené do 2 folder-ov, src a dist

  • V dist sa nachádza aktuálne najnovšie CSSko (voči buildovaniu vždy nového). Initial setup sa zmenil z buildovania všetkého len na spustenie ./buildcss.sh setup, ktorý skopíruje najaktuálnejšie CSSka z votrfront/css/dist do votrfront/static.

  • Vo votrfront/css/dist sa nachádzajú 2 súbory, bootstrap.custom.css a votr.css.

    • bootstrap.custom.css je Bootstrapový základ, ktorý sa skoro nikdy nemení. Ak by ho predsalen bolo treba zmeniť (napr. treba novú časť Bootstrapu pridať ktorá bola cut-nutá), je na to ./buildstatic bootstrap.
    • votr.css je všetko custom CSS (buildnuté pomocou ./buildcss). Spolu s custom Votr SASSom je tam aj nejaké minimum ktoré je potrebné na buildnutie, ako priečinok mixins, _variables atď. Toto je CSS buildnuté z main.scss, ktoré je v drvivej väčšine prípadov relevantné.

JS commit (-y) robia zopár vecí.

  • je pridaný build system s normálnymi package-mi. Pôvodný skript bol dosť nebezpečný, pretože využíval internú štruktúru package-ov, ktorá nie je stabilná. Takisto robil len to, že concate-oval všetky JS zdrojáky, čo je trochu nebezpečné pri používaní var. Nový build system sa skladá z 2 krokov - yarn build buildne produkčný bundle s prakticky všetkými optimalizáciami čo sa dá, yarn dev builduje dev bundle so sourcemapami a ostatnými srandami.

  • kód je komplet refaktornutý na ES6. Nenachádza sa v zdrojáku už ani jeden var, React componenty využívajú ES6 classes a podobné srandy. Snažil som sa držať podporu s IE 11, preklikal a vyskúšal som všetko čo sa dalo a nenašiel som nijakú nekompatibilitu.

  • s predchádzajúcim bodov súvisí aj tento - všetky lodash funkcie boli nahradené ES6 ekvivalentami. Cena je trošku nižšia čitateľnosť, čo si ale myslím, že nerobí rozdiel, lebo aj tak si človek vždy musí pozrieť lodash-ovú dokumentáciu, že čo to vlastne robí, takto to je pochopiteľné priamo z kódu. Nechal som ale v projekte celý lodash build pipeline, takže ak niečo bude treba, dá sa to optimálne použiť, teda není includenutý celý lodash, ale tree-shaknutá verzia.

  • opäť nadväzujúc na predchádzajúci bod, veľkosť bundle-u bola celkom slušne zmenšená, a to na 94.21kb (gzip) z 117.752kb (zip) (@TomiBelan tak ako si si želal, nepridával som zbytočné dependancies :P ). Toto bolo docielené spomínaním nahradením lodash-u nativnými funkciami, upgradeom dependancies (React 16 je ~30% menší) a tree-shake-ovaním všetkého možného. Taktiež, teraz je už len jeden bundle a nie 10 súborov.

  • pridal som do projektu eslint a refaktorol som celý projekt na jeden code style. Poprosil by som ešte človeka čo toto číta a vyzná sa, nech pozrie zvyšné errors čo tam ostali, väčšina je o nepoužitých premenných, treba pozrieť, či sa dajú nahradiť _ alebo nejako inak vynechať. Ak sa toto vyrieši, môže sa setupnúť niečo na nejako CI servery aby to vždy zbehlo na PR nech sa konzistencia udrží.

Jediný problém, čo som mal bol VotrDevel plugin, naozaj som nedokázal prísť na to, že čo vlastne robí, tak ak to bude niekomu chýbať a je ochotný mi to vysvetliť, tak to dorobím, prípadne môže to dorobiť sám.


This change is Reviewable

@TomiBelan
Copy link
Member

Ach beda, prebeda. Zanechajte vsetku nadej, vy, ktori sem vstupujete!

Musim ta varovat: o toto sa snazim uz asi styri mesiace a este som nenasiel riesenie co by upokojilo moj priserny perfekcionizmus. Tym sa nenechaj odradit! Len s tym rataj, ze asi budem mat vela pripomienok a asi to bude chciet viacero iteracii.

Idem zacat citat jednotlive commity a popri tom tu mozno budem pisat na co som prisiel v mojich vlastnych pokusoch.

@kubik369
Copy link
Collaborator Author

kubik369 commented Mar 3, 2018

@TomiBelan Som na to pripravený, ďakujem za ochotu :)

@TomiBelan
Copy link
Member

Tak tu je prva salva mojich otazok. Je ich take mnozstvo ze ich radsej ocislujem. Nateraz ma len zaujima co si o nich myslis - menit kod je este skoro, plan sa este moze velakrat zmenit.

CSS

Tym CSS zmenam vseobecne velmi nerozumiem.

[1] Preco je vystup rozdeleny na votr.css a bootstrap.custom.css?

[2] Preco je vystup aj vo votrfront/css/dist aj vo votrfront/static? Preco je jeden commitnuty v gite a druhy nie?

[3] Odkial pochadzaju votrfront/css/src/mixins a preco existuju?

Tipujem ze odpoved na aspon niektore z tychto otazok je snaha nezavisiet na internej strukture npm baliku bootstrap-sass. Normalne by som suhlasil - vacsina npm kniznic sa ma pouzivat iba cez require('meno-baliku') a cokolvek viac je private detail na ktory sa nemame spoliehat... ale bootstrap-sass mi nepride ako tento pripad. Navyse vyzera ze buildcss.sh aj tak taha veci z node_modules/bootstrap-sass/assets.

[4] Je css watching prec? Mas nejaky plan co s nim?

[5] Mas nejaky nazor o libsass-python vs node-sass? (moj: oba su nanic, z roznych dovodov)

JS (webpack)

[6] Ked sejvnem subor a vzapati reloadnem prehliadac, mozno dostanem staru verziu. Stary watcher aspon ukazal "buildstatic failed!" cim mi povedal ze musim pockat a reloadnut znova. Vieme nieco aspon take dobre a mozno lepsie? (Napr ze by ten reload cakal kym sa dokompiluje.) Dva mozne sposoby: bud webpacku povedat nech vyrobi nejaky subor ze ci prave kompiluje alebo nie (co neviem ako sa da), alebo mozno proxyovat webpack-dev-server (ktory mi nefungoval ale mozno odvtedy opravili).

[7] prologue.js bol oddeleny preto, aby aj stare prehliadace co nepoznaju ES5 getter syntax ukazali aspon ten error. Funguje tvoja verzia v IE6-10? Mozno stale treba samostatny prologue (ale mozno ho netreba kompilovat).

[8] Ked boli kniznice zvlast, malo to tu prijemnu vlastnost ze som pekne videl aky je votr samotny malicky oproti knizniciam :-) Ale vyzera ze mat samostatne kniznice uz nie je v mode a asi ich naozaj chceme bundlovat. Zaujimalo by ma ci sa da tato vlastnost zachovat - napriklad code splittingom na "votr.bundle.js" a "libs.bundle.js". Alebo aspon vypisovanim tejto statistiky.

[9] Ako s webpackom ludia robia konzolovy debugging? Napr ako by som inspectol obsah requestCache z ajax.js? Doteraz som mal Votr.webpackRequire ako unikovy vychod, ale aj to bol dost hack. Mal som napad spravit plugin ktory pre kazdy modul automaticky spravi import * as x from './ajax'; window.votr_ajax = x;. Existuje nieco standardnejsie?

JS (zvysok)

[10] Asi je dobry napad mat bud package-lock.json alebo yarn.lock. Ale ktory? Videl som https://yarnpkg.com/blog/2017/05/31/determinism/ ale neviem co si o tom mysliet. Samozrejme kazdy si moze vybrat co chce pouzivat lokalne, ale ktory tool zoficialnit. Mam dojem ze npm je "standardnejsi" ale na druhu stranu npm developeri tusim neprekypuju kompetenciou, tak neviem.

[11] Neviem co s lodashom. Parkrat som sa ho uz pokusal zbavit, ale vzdy mi nakoniec prisiel prilis uzitocny aby som ho vyhodil. Spravne rozumiem ze vdaka tomu babel pluginu sa aj tak zabundluju len tie lodash funkcie, co pouzivame? Ak ano, tak mozno preco nie pouzivat ho dalej. Zvlast _.sortBy a _.map mi prisli ako najvacsie downgrady v citatelnosti, tie zvysne sa daju prezit (ale zase az tak neprofitujeme lebo ich lodashovy vyznam je zjavnejsi).

[12] Zaujimalo by ma, ci ma "browsers": ["last 2 versions", "IE >= 11"] vobec nejaky efekt. Babel sa strasne tvari ako to mozes podrobne nakonfigurovat ale tipujem ze ked mame IE >= 11 aj tak musi prekladat kazdu ES6 ficuriu. Ach jo...

[13] nvm je sympaticka alternativa ale neviem ci ju prezentovat ako hlavnu moznost. Na produkcnom serveri votr.uniba.sk asi nvm nepouzijeme, kedze funguje cez bash profile (skor asi ten nodesource repozitar).

[14] React.Component nepodporuje mixins, takze sa ich bude treba nejako zbavit (asi prepisat na higher order component). To by mohla byt samostatna zmena, ci uz pred touto alebo po nej (t.j. ze zatial pouzijeme require('react-create-class') a potom sa ho zbavime).

[15] Kedze (na rozdiel od CSS) nevyzera ze pridavas votrfront/js/dist, preco vlastne ten presun z votrfront/js do votrfront/js/src?

[16] Vyzera ze veci nie su uplne dobre rebasnute na najnovsie zmeny v masterovi. Napriklad som si vsimol ze v LogViewer.js nie je benchmark podpora a ze ovce.js je spat, ale asi sa najdu aj ine veci. Ale rozumej: nechcem po tebe aby si teraz rucne nasiel a patchol vsetky tie zmeny. Horsie: vymysli ktore z tych zmien v .js sa daju zautomatizovat a tym ma presvedcit ze su OK ;-)
Pre inspiraciu, cely eedc8b2 vznikol automaticky (aj ked to je dost extremny pripad, tu si skor predstavujem kroky tvaru "spustil som eslint" a "pouzil som ten a ten regex")

[17] Co si myslis o eslint vs prettier (resp. vs oboje ale eslint iba na nesyntakticke veci)?

Mal by som brzdit. Nateraz staci.

Ak po tom vsetkom stale chces pokracovat, mas moj nehynuci obdiv a vdacnost. Ak si si to rozmyslel, nebudem to brat ako ujmu na cti ;-)

@TomiBelan TomiBelan self-requested a review March 4, 2018 22:03
@kubik369
Copy link
Collaborator Author

kubik369 commented Mar 4, 2018

CSS

[1] Pointou tohoto rozdelenia je mať trošku lepšie rozdelené čo je reálne custom CSS napísané pre Votr a čo je Bootstrap samotný s nejakými setnutými premennými. Túto ideu som v retrospektíve nie úplne dotiahol, lebo som to nechal v rovnakom source foldery :/

[2] static nebol commitnutý v gite ani pôvodne a zatiaľ som nemal odvahu moc do toho pythonu štuchať aby som zmenil odkiaľ to tie CSS berie a tak trochu som čakal na ohlas tejto zmeny.

Pointou dist je mať CSS aktuálnej verzie pripravené a nemusieť ho buildovať pokiaľ človek nechce ísť nič s CSS robiť.

[3] mixins priečinok sa tam nachádza, pretože niektoré Bootstrap mixins sú použité v SASSe main.scss (aspoň to bol môj záver keď som sa to snažil buildnúť)

[3.5]
Áno, je to pokus to nejako od toho odtrhnúť od tohoto spôsobu. Áno, zatiaľ to tak stále je, viz. koniec bodu 2. Interná štruktúra je jedna vec, potenciál zmazania balíku druhá.

[4] Nebudem klamať, nejako som si nespojil to, že ten buildstatic skript pri každej zmene zbehne s tým, že vlastne aj buildne CSSka (viem, stupid). Riešenia, ktoré mi napádajú sú ponechanie toho watchovania v serveri a buildovanie CSSka z toho, mať nejaký taskrunner ktorý bude robiť všetko, alebo (nedajbože!) mať CSS v bundli a includenuté vo webpack build pipeline. Osobne si myslím, že posledné je najlepšie alternatíva z pohľadu dev experience, ale je aj najviac taxing na zmenu. Asi ma budeš mať za heretica, ale mám radšej pri Reacte veci ako napr. react-bootstrap - majorly (úplne?) bez side effects, ktoré sú
nezistiteľné z kódu + niečo štýlu postcss/styled-components/proste niečo.

[5] Tiež si myslím, že obe sú na nič, už len z dôvodu, že na trojstenwebe som sa snažil niečo s tým vymyslieť a dokumentácia sa mi zdala po hľadaní atrocious od bodu čo som nevedel ani CLI options nájsť poriadne. Som ochotný sa v tom potopiť a preskúmať to.

JS webpack

[6] Chápem, ako by to mohol byť problém, môžem sa po niečom pozrieť, z tvojich alternatív mi príde viac plausible tá druhá. Z mojich skúseností by si ale musel mať podľa mňa dosť insane workflow (alebo dosť pomalý stroj/fakt vysoké očakávania) aby si stihol tak rýchlo alt-tabnúť a reloadnuť. Na mojej i5-3320M, čo je už 5-6 rokov stará bol incremental build ~700-800ms, osobne nedokážem, pokiaľ sa fakt nesnažím, tak rýchlo executnuť (Ctrl + S) + (Alt + Tab) + (F5 / Ctrl + R).

To, že by mohol build failnuť sám o sebe by malo byť možné vidieť už z editora (eslintu) - určite je aj plugin na VIM prinajhoršom.

[7] Nie je na takéto veci <noscript>, prípadne nejaký UA string checker? Nemám to momentálne ako otestovať, ale ten bundle by mohol (?) vedieť prežiť do bodu keď sa dostane k tomu bundlenutému prologue.js, prípadne by sa možno dal dať vyššie v hierarchií? Ak sa mýlim a vôbec tomu tak nie je, tak ten samotný prologue znie fajn.

[8] I got you all covered. Asi si to nemal pustené, ale keď pustíš webpack, tak je tam aj plugin
BundleAnalyzer a ten ukazuje takú
cool infographic veľkosti balíkov (spoiler, Votr je väčší ako by sa zdalo z pôvodnej veľkosti ;-) ).

[9] Toto mi príde ako práca pre debugger, buďto v Chrome alebo standalone. Ak povieš čo všetko od toho chceš, môžem sa na to pozrieť.

JS zvyšok

[10] Tiež mám pocit, že npm developerom nebolo veľa kompetencie danej do vienku. Mám zatiaľ veľmi dobré skúsenosti s Yarnom, hlavne v usability a rýchlosti. Ešte som nemusel nejako extra využiť tie alleged deterministické features. Jediné 2 výhody, čo vidím v npm je a) bundle-nuté priamo s Node-om, b) npx (dá sa ľahko nahradiť, ale je to šikovné).

[11] Áno, ten babel plugin nahraduje import _ from 'lodash' niečím štýlu import omit from 'lodash/omit', takže by to malo byť tree-shakenuté. Sú ešte potom balíky štýlu lodash.omit, ale keďže som to nakoniec celé nahradil, nedostal som sa k porovnaniu.

Čitateľnosť týchto dvoch prístupov je podľa mňa vec názoru. Moje stanovisko je takéto:

Na prvý pohľad je jasné, čo robí napr _.sortBy, pozriem, vidím, sortuje to toto a hento. Problém ale prichádza, keď sa chce niečo upravovať. Naozaj ma to zabolelo, keď som to prepisoval. Jedná sa o to, že nevieš, čo je tá collection ktorú sortuješ. Lodash dokumentácia nepomáha, lebo majú funkcie pre všetky collections v jednom. Je to podľa mňa dosť dvojsečná zbraň.

Druhý problém čo s tým osobne mám - moc často ale lodash nepoužívam - je ten, že aj tak som musel čítať ich dokumentáciu, aby som sa dopátral k tomu, čo je napr. druhý parameter _.sortBy. Hint v editore, že to je iteratee fakt není nápomocný a to že vracia niektorú hodnotu z prvku, ktorý dostane je trochu guesswork sám o sebe. Napr. by som očakával, že druhý parameter je komparátor.

// EDIT ešte ma napadlo mať funkciu/súbor s funkciami, ktorý robí to čím som to nahradil a schováva to za nejaké pekné meno.

[12]
Niektoré ES6 veci prekvapivo IE11 podporuje, nemal som zatiaľ prostriedky na skontrolovanie niečoho staršieho, ale mám podobné podozretie, toto bol default config.

[13]
NVM by mal byť len jeden skript, takže by sa to malo dať použiť aj samo o sebe. Prinajhoršom vieš sourcenuť ten script len v jednej session, nainštalovať binárku a dať path k tej, to je vlastne pointa toho celého.

[14]
V tomto bode sa už cítim hlúpo a fakt neprofesionálne, že som to lepšie necheckol. Tvoje riešenie znie dobre.

[15]
Aby boli zdrojáky oddelené od všetkých configov a iných vecí, je z toho fakt bordel ak to je v kope. Bolo to v podstate takto isto aj predtým, len som to pushol o jednu úroveň nižšie.

[16]
Toto je výsledok toho že som to mal forknuté a moc som s tým ešte nerobil, tak to catch-upovanie zmien za rok bolo trochu bolestivé. Pozriem sa na to a nejako to skúsim vyriešiť.

[17]
Zatiaľ som sa veľmi nehral s prettier, koncept znie dobre, neexperiencol som ešte exekúciu, môže sa skúsiť. Osobne si ale myslím, že prettier je dobré mať ako samotný commit na konci a na CI stále behať eslint.

Fu, obdivujem ťa, že si mal nervy toľko otázok napísať, muselo to trvať nekonečne dlho, len odpovede som písal asi hodinu a pol :D.

@TomiBelan
Copy link
Member

[1] Hmm ale ma to nejake vyhody? Ked uz v JS rusime moj staromodny pristup mat vsetky kniznice zvlast a vsetko ideme skombinovat do jedneho suboru, preco CSS ide opacnym smerom? ;-)
Navyse, priamo na https://github.com/twbs/bootstrap-sass odporucaju v tvojom sass subore napisat @import 'bootstrap-custom';.

[2] Neviem ci je az taka vyhoda ze ked nemenis CSS tak ho nemusis buildovat. sassc bezi rychlo, instalujeme ho takcitak (lebo je v requirements.txt) a od 54618c1 uz instalacia prilis neboli (existuju wheels a pip nemusi kompilovat libsass).
A mat build vystupy commitnute v gite ma svoje nevyhody... neni to v diffoch velmi citatelne, treba to drzat synchronizovane (co sa bez CI lahko zabudne) a komplikuje to merging/rebasing.
WDYT? (t.j. "what do you think?")

[3, 3.5] Ked vravis "zatial": existuje sposob ako sa totalne odtrhnut od internej struktury bootstrap-sass? Teraz na nej veselo dependujeme - aj ked len pri kompilacii bootstrap.custom.css. Vidim dve sebakonzistentne filozofie: bud vendorovat cely bootstrap-sass, alebo z neho nemat v nasom gite nic. Teraz je to nejako divne medzi tym (a asi to ma nevyhody obojeho).

[4] Poznam tieto moznosti:

  • Nechat watchstatic.py, iba pre CSS (fuj).
  • Pouzit samostatny tool, ktory sleduje ked sa zmenia zdrojaky a zavola samostatny rebuildovac (ci uz pythonovy alebo nodeovy). Prehladal som mozno aj vsetky tieto tooly a neveril by si ake su hrozne. Nikto nechape ze ak sa vstup zmeni kym bezi kompilator, treba kompilator spustit znovu (a ten stary bud zabit alebo pockat kym skonci). Konecne som nasiel jeden dobry: onchange.
  • Pouzit webpack - to by sa dalo, aj ked to komplikuje pripadny buduci prechod na iny build system.

[5] Usetrim ti pracu a zacitujem description z mojho nedokonceneho commitu:

Build style.css with webpack and node-sass.

Pros:

  • buildstatic.sh no longer needs Python or --env.
  • libsass is removed from requirements.txt, so anketa won't install it.
  • We can use url-loader and such.
  • We can use webpack-dev-server or --watch.

Cons:

  • node-sass downloads binaries directly from github. Probably not a real problem, but it feels much dirtier than wheels.
  • node-sass has a sad amount of dependencies. :( But as a consolation, python-libsass doesn't strip its .so, so the total used disk space is pretty similar.

Trochu skripem zubami ale asi som ochotny node-sass akceptovat. (Zvlast kedze samotny webpack 4 ma absurdnych dependencies tusim podstatne viac.)

[6] So starym build systemom sa mi to urcite stavalo, ale mozno ze incremental builds to riesia. Ak nie, pohram sa s webpack pluginmi, to nejako musi ist.

[7] Problem je s IE<9, ktore poznaju JS ale nepoznaju getter syntax. Vyhodia syntax error uz pri parsovani a nevykona sa nic. :/ Preto bol prologue.js zvlast.

[8] Neat. Aj ked vyzera ze ukazuje velkost pred minifikaciou.
Prichytil si ma, naozaj som to najprv nespustil... ;-)

[9] Debugger je fajny ked mam konkretny breakpoint, ale skor by ma zaujimalo ako len tak z konzoly volat nahodne funkcie alebo obzerat nahodne globalne premenne. Najcastejsie sa mi to hodi so spominanym requestCache. Ale vseobecne mi pride uzitocne moct na konzole napisat nieco ako require('coursesStats').currentAcademicYear() a takto tie fcie interaktivne testovat.

[10] Hmm, musim sa o tom este zamysliet. Mozno spravit prieskum kolko balikov uvidim s package-lock.json a kolko s yarn.lock.
Ale aspon jednu prekazku yarn prekonal: ma deb repo, takze sa bude dat pekne nainstalovat aj na votr serveri.

[11] Neviem ci rozumiem a neviem ci suhlasim. TODO: napisat zmysluplnejsiu odpoved.

[12] OK.

[13] Na prod serveri existuje skript deploy-votr, ktory mimo ineho robi sudo -Hu votr ./votrfront/buildstatic.sh --env=venv. Po novom by asi robil nieco ako sudo -Hu votr yarn build. Kedze sudo afaik zacina od prakticky prazdneho environmentu, pchat do jeho commandlinu aktivaciu nvm je dost krkolomne a pomerne skarede. Linkovat node do /usr/local/bin ditto.
Nic proti nvm, kludne ho mozeme spomenut ako moznost, ale pride mi uzitocne iba ak: 1. mas hlupe distro (t.j. debian/ubuntu), 2. nebezime na prod serveri, 3. nechces robit systemwide zmeny (napr lebo nemas roota).

[14] Pohoda.

[15] Skoda ze ten presun komplikuje review (git nechape ze su premenovane) a kazi blame. Kvoli nim sa trochu priklanam k staremu layoutu - aj ked urcite tiez nie je idealny.

[16] OK.

[17] Tiez to skusim preskumat a porovnat ich. Ale mas pravdu ze mozno je lepsie formatovacie zmeny oddelit.

@kubik369
Copy link
Collaborator Author

kubik369 commented Mar 11, 2018

Ospravedlňujem sa za dlhšie čakanie na pokračovanie našeho románu.

[1] Hmm ale ma to nejake vyhody? Ked uz v JS rusime moj staromodny pristup mat vsetky kniznice zvlast a vsetko ideme skombinovat do jedneho suboru, preco CSS ide opacnym smerom? ;-)
Navyse, priamo na https://github.com/twbs/bootstrap-sass odporucaju v tvojom sass subore napisat @import 'bootstrap-custom';.

Má to výhodu, že máš trošku lepšie rozdelené že čo je custom, ale máš pravdu, asi to je trochu nadbytočné, revertnem to.

[2] Neviem ci je az taka vyhoda ze ked nemenis CSS tak ho nemusis buildovat. sassc bezi rychlo, instalujeme ho takcitak (lebo je v requirements.txt) a od 54618c1 uz instalacia prilis neboli (existuju wheels a pip nemusi kompilovat libsass).
A mat build vystupy commitnute v gite ma svoje nevyhody... neni to v diffoch velmi citatelne, treba to drzat synchronizovane (co sa bez CI lahko zabudne) a komplikuje to merging/rebasing.
WDYT? (t.j. "what do you think?")

Myslím si, že na tom až tak nezáleží, má to svoje výhody a nevýhody. Keďže by sa potenciálne chcelo prejsť na react-bootstrap, tak si myslím, že to je jedno. Skúsim to vymyslieť tak, aby to buildovalo všetko naraz s niečím, ako napr. gulp. Čo si o niečom takom myslíš ty?

[3, 3.5] Ked vravis "zatial": existuje sposob ako sa totalne odtrhnut od internej struktury bootstrap-sass? Teraz na nej veselo dependujeme - aj ked len pri kompilacii bootstrap.custom.css. Vidim dve sebakonzistentne filozofie: bud vendorovat cely bootstrap-sass, alebo z neho nemat v nasom gite nic. Teraz je to nejako divne medzi tym (a asi to ma nevyhody obojeho).

Dá sa od nej odtrhnúť, keď sa použije react-bootstrap, trochu škaredšia verzia je mať všetky tie zdrojáky v repe, ale aj to je lepšia alternatíva na spolahlivosť ako to kopírovať.

[4] Poznam tieto moznosti:
Nechat watchstatic.py, iba pre CSS (fuj).
Pouzit samostatny tool, ktory sleduje ked sa zmenia zdrojaky a zavola samostatny rebuildovac (ci uz pythonovy alebo nodeovy). Prehladal som mozno aj vsetky tieto tooly a neveril by si ake su hrozne. Nikto nechape ze ak sa vstup zmeni kym bezi kompilator, treba kompilator spustit znovu (a ten stary bud zabit alebo pockat kym skonci). Konecne som nasiel jeden dobry: onchange.
Pouzit webpack - to by sa dalo, aj ked to komplikuje pripadny buduci prechod na iny build system.
[5] Usetrim ti pracu a zacitujem description z mojho nedokonceneho commitu:
Build style.css with webpack and node-sass.
Pros:
buildstatic.sh no longer needs Python or --env.
libsass is removed from requirements.txt, so anketa won't install it.
We can use url-loader and such.
We can use webpack-dev-server or --watch.
Cons:
node-sass downloads binaries directly from github. Probably not a real problem, but it feels much dirtier than wheels.
node-sass has a sad amount of dependencies. :( But as a consolation, python-libsass doesn't strip its .so, so the total used disk space is pretty similar.
Trochu skripem zubami ale asi som ochotny node-sass akceptovat. (Zvlast kedze samotny webpack 4 ma absurdnych dependencies tusim podstatne viac.)

Asi ako som spomínal, niečo skúsiť pospájať s nejaký task-runnerom ako gulp (samozrejme sú iné, spomínam tento lebo s tým som robil a definuje mi to kategóriu :D ).

[6] So starym build systemom sa mi to urcite stavalo, ale mozno ze incremental builds to riesia. Ak nie, pohram sa s webpack pluginmi, to nejako musi ist.

Skúsim sa na to aj ja pozrieť. Asi by to poriešil webpack-dev-server a v serve.py dať inú adresu bundle-u, keď sa buildí dev bundle.

[7] Problem je s IE<9, ktore poznaju JS ale nepoznaju getter syntax. Vyhodia syntax error uz pri parsovani a nevykona sa nic. :/ Preto bol prologue.js zvlast.

Skúsim to nejako otestovať. Btw, Browserstack je free pre OSS projekty. Možno by sa to hodilo ak chceš držať podporu IE. Ja si osobne myslím, že by bolo dobré pozrieť analytics, či je dosť veľký počet ľudí na to aby sa nad tým oplatilo rozmýšľať a ak nie, tak proste vyriešiť ten prologue a killnuť všetky IE.

[8] Neat. Aj ked vyzera ze ukazuje velkost pred minifikaciou.
Prichytil si ma, naozaj som to najprv nespustil... ;-)

Pokiaľ dáš yarn build, urobí to len jeden build a zasekne sa to s tým pusteným inspectorom, tam vídiš minifikované veľkosti.

[9] Debugger je fajny ked mam konkretny breakpoint, ale skor by ma zaujimalo ako len tak z konzoly volat nahodne funkcie alebo obzerat nahodne globalne premenne. Najcastejsie sa mi to hodi so spominanym requestCache. Ale vseobecne mi pride uzitocne moct na konzole napisat nieco ako require('coursesStats').currentAcademicYear() a takto tie fcie interaktivne testovat.

No, vieš si tie premenné ktoré budeš chcieť obzerať proste exposenuť v dev bundle-i, ale napr. v chrome devtools sa dajú tuším sledovať premenné aj bez breakpoints

[10] Hmm, musim sa o tom este zamysliet. Mozno spravit prieskum kolko balikov uvidim s package-lock.json a kolko s yarn.lock.
Ale aspon jednu prekazku yarn prekonal: ma deb repo, takze sa bude dat pekne nainstalovat aj na votr serveri.

OK :)

[11] Neviem ci rozumiem a neviem ci suhlasim. TODO: napisat zmysluplnejsiu odpoved.

TODO nevyšlo. Snažil som sa povedať len to, že komplexita s použitím lodashu bola v tom, že trebalo aj tak čítať dokumentáciu. Presunul som túto komplexitu do toho, že sa musíš nachvíľku zastaviť, čo to celé robí, ale je jasnejšie, čo to vyplúva a ako to funguje.

[12] OK.

OK :)

[13] Na prod serveri existuje skript deploy-votr, ktory mimo ineho robi sudo
-Hu votr ./votrfront/buildstatic.sh --env=venv. Po novom by asi robil nieco ako
sudo -Hu votr yarn build. Kedze sudo afaik zacina od prakticky prazdneho
environmentu, pchat do jeho commandlinu aktivaciu nvm je dost krkolomne a
pomerne skarede. Linkovat node do /usr/local/bin ditto. Nic proti nvm, kludne
ho mozeme spomenut ako moznost, ale pride mi uzitocne iba ak: 1. mas hlupe
distro (t.j. debian/ubuntu), 2. nebezime na prod serveri, 3. nechces robit
systemwide zmeny (napr lebo nemas roota).

Ak vieš nainštalovať nejaký rozumne recent node, tak ako chceš. Ja si zase myslím, že je škaredé (a trochu divné), že musíš robiť sudo na deploy :D Možno by sa mohlo setupnúť CI a urobiť nejaký pekný deploy k tomu.

[14] Pohoda.

Tak dorobím

[15] Skoda ze ten presun komplikuje review (git nechape ze su premenovane) a kazi blame. Kvoli nim sa trochu priklanam k staremu layoutu - aj ked urcite tiez nie je idealny.

Skúsim git prinútiť aby ich pohol a nie zmazal.

[16] OK.

OK.

[17] Tiez to skusim preskumat a porovnat ich. Ale mas pravdu ze mozno je lepsie formatovacie zmeny oddelit.

OK.

@TomiBelan
Copy link
Member

Narychlo ciastocna odpoved.

[2,3,4]
Ak spravne rozumiem, react-bootstrap riesi iba JS cast a cele CSS je stale na nas. Mozno nam pomoze zbavit sa jquery, ale problemy s tym ako buildovat CSS asi neovplyvni.
Ako hlavnu nevyhodu gulpu vnimam, ze jeho watch mode nie je integrovany s webpackovym watch mode. gulp config je retaz v podstate bezstavovych fcii co nejako lubovolne transformuju data, ale webpack je prudko stavovy a robi svoj vlastny watching. Preto mi pride nepravdepodobne ze ich pojde pekne a bezbolestne skombinovat. Kludne to vyskusaj ale moj tip je ze je to slepa ulicka.

[7] https://netrenderer.com/ je tiez fajn (a mozno rychlejsi). Podporujeme iba IE>=10, a ak by robili problemy, som ochotny IE10 and/or IE11 dropnut. Ale aj na nepodporovanych IE urcite chceme aspon error message.

[11] To TODO som myslel pre mna :-) Na rozumnejsej odpovedi sa stale pracuje. Zatial aspon dodam: mozno ma zmysel rozhodnut sa o kazdej pouzitej fcii zvlast, ci ju chceme. A mozno by to slo oddelit do samostatneho PR. ;-)

[13] Len drobna oprava: deploy skript so sudo neziskava roota, iba z usera tomi ide na usera votr.

FYI: vyzera ze nova ais2-beta spravila velke zmeny v UI markupe, takze moja prva priorita je teraz updatnut aisikl nech to vie parsovat. Snad budem stihat aj tento review, aj ked mozno menej aktivne nez som si predstavoval.

@kubik369
Copy link
Collaborator Author

Rýchla oprava mojej odpovede:

[2,3,4]
Ak spravne rozumiem, react-bootstrap riesi iba JS cast a cele CSS je stale na nas. Mozno nam pomoze zbavit sa jquery, ale problemy s tym ako buildovat CSS asi neovplyvni.
Ako hlavnu nevyhodu gulpu vnimam, ze jeho watch mode nie je integrovany s webpackovym watch mode. gulp config je retaz v podstate bezstavovych fcii co nejako lubovolne transformuju data, ale webpack je prudko stavovy a robi svoj vlastny watching. Preto mi pride nepravdepodobne ze ich pojde pekne a bezbolestne skombinovat. Kludne to vyskusaj ale moj tip je ze je to slepa ulicka.

Pozabudol som, react-bootstrap + (postcss + cssloader) (alebo čo teraz fičí, ja si pamätám toto combo). To je integrované s webpackom, samozrejme, obnáša to jemný refaktor, ktorý som ochotný spraviť :)

@TomiBelan
Copy link
Member

[4] FYI, vyzera ze extract-text-webpack-plugin (ktory bude treba ak chces buildovat css cez webpack) este nepodporuje webpack 4 :/

@TomiBelan
Copy link
Member

Ako sa dari? Asi si medzitym nemal vela casu ;-) Len som chcel dodat ze kludne pushuj aj nehotove alebo nefunkcne verzie. Na konci to sice bude chciet peknu commitovu historiu, ale to pocka.


Zacina sa rysovat zoznam relativne nezavislych uloh:

  • ta velka: prejst na package.json a prerobit JS na webpack (a vyzera ze asi aj CSS) (a watching) (a prologue) (atd)
  • prejst na novy React a z React.createClass na React.Component
  • prejst na bootstrap 4
  • prejst na react-bootstrap (asi)
  • vylepsit formatovanie (asi)
  • eliminovat lodash (mozno)

Mas nejake preferencie, na ktore z nich sa chces pozriet a v akom poradi?


Co som medzitym vypatral...

[10] Yarn u mna brutalne vyhrava a to hned z troch dovodov:

  1. Ked nic netreba robit, skonci za 0.5s (npm za 3s).
  2. Podporuje resolutions. Vsetky baliky ma stale mimoriadne irituju tym kolko maju uplne zbytocnych a nepouzitych dependencies a tym zeru serverovy disk a sietovy traffic. (Napriklad: vedel si ze tento PR dependuje na babel 6 aj 7? Ugh, ludia.) S resolutions sa to da ohackovat a nahradit a velke zbytocne baliky za falosne prazdne. (Aj ked to asi nebol primarny umysel yarn autorov...)
  3. Vie --cwd. Vid nizsie.

[14] Zistil som ze class properties sa daju zapnut aj v Babel 5, takze prechod na React.Component moze byt uplne nezavisly od prechodu na novsi Babel. Yay.

[15] Vidim tri moznosti ako spravit adresarovu strukturu.

a) votrfront/js/src/*.js + votrfront/js/package.json
b) votrfront/js/*.js + votrfront/package.json
c) votrfront/js/*.js + package.json

Argument v prospech c) oproti a) a b) je takyto: neviem ci to len ja, ale nerad robim cd. Je to taka skareda globalna premenna pre cely moj terminal ;) Nemozem spustat prikazy z historie co si myslia ze su inde, a podobne. V tomto ohlade je dost krkolomne instalovat JS zavislosti s cd votrfront/js, yarn, cd ../.., a buildovat s cd votrfront/js, yarn dev. Zato keby bol package.json v koreni, ziaden problem.

Argument proti c) je ze package.json aj node_modules sa tykaju iba votrfrontu a mali by byt v nom. Polahcujuca okolnost je ze yarn vie --cwd, takze funguje napr yarn --cwd votrfront/js dev. Keby nevedel, argument v prospech c) by bol (aspon pre mna) vyrazne silnejsi. Aj tak je to slabe, lebo adresare ako logs a sessions sa teoreticky tiez tykaju iba votrfrontu a pritom su v koreni.

Vyhoda a) oproti b) je ze subory okolo JS buildu su oddelene od votrfront/*.py a teda je tam mensi bordel. Vyhoda b) oproti a) je ze netreba presviedcat git ze je to len move a nie add+delete, a ze zostane fungovat git blame a podobne. Zatial mi to stale pride v prospech b) (hlavne lebo mi ten bordel nepride taky velky). Ale medzi b) a c) sa neviem rozhodnut. Co si myslis ty, je lepsie a) ci b) ci c)?

@TomiBelan TomiBelan mentioned this pull request May 16, 2018
@TomiBelan
Copy link
Member

Mám dobrú a zlú správu. Dobrá správa je, že jeden júlový týždeň som sa nudil, povedal som si že skúsim celý build system vyhodiť a napísať odznovu (berúc ohľad na [1]-[17] vyššie), a na moje neskutočné prekvapenie som dosiahol prakticky totálny úspech. Zostáva už len upratať git históriu, inak v podstate všetko funguje presne ako má, a lepšie ako kedy predtým.

To je zároveň tá zlá správa. Nad týmto PR sa zbierajú búrkové mračná. Cením si na ňom, že pomohol vykryštalizovať problémy čo treba vyriešiť, ale nevyzerá pravdepodobne že bude mergnutý. Čo je škoda, lebo s kvalitou tvojho kódu ťa chcem vidieť niekde vysoko na https://github.com/fmfi-svt/votr/graphs/contributors :p

V blízkej dobe doupratujem svoj rewrite, pushnem ho ako summerbuild a spíšem ako rieši [1]-[17]. Ak náhodou práve máš čas, poteším sa aj ak mi spravíš code review.
Zvyšné úlohy (bootstrap 4, react-bootstrap alebo reactstrap, prettier, eslint) sú stále otvorené a určite privítam tvoju pomoc.

@TomiBelan
Copy link
Member

Sľubovaný rewrite už visí na #127. Pre budúcich archeológov zhrniem, ako summerbuild odpovedá na otázky z tohto threadu:

[1], [2], [3], [4], [7], [8], [16]: summerbuildu sa to netýka, nechal som to po starom.
[5]: summerbuild používa node-sass. Dôvody sú v fa4cd2c.
[6]: front.py čaká. Slabé dôvody prečo nie webpack-dev-server sú v da25adb.
[9]: zostalo nevyriešené. Asi časom exportnem aspoň RequestCache.
[10]: summerbuild používa yarn. Dôvody sú vyššie a sú zhrnuté v f8ee773, aj keď nakoniec nie sú také pádne a npm by fungovalo tiež.
[11]: lodash som zatiaľ nevyhodil. FWIW, sorting.js je podstatne kratší odkedy používa _.orderBy.
[12]: summerbuild používa "browserslist": "defaults, not dead", ale netuším či je to dobré nastavenie.
[13]: nvm nezmieňujem (zvlášť lebo aj yarn samotný odporúča inštaláciu z .deb) ale kto chce, nech ho kľudne používa.
[14]: už vybavené dávnejšie v e78e9cf a da3e012.
[15]: vybral som si c), kvôli mojim dôvodom vyššie. Popísané v f8ee773.
[17]: summerbuild zatiaľ nemá ani eslint ani prettier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants