Skip to content

Conversation

@TomiBelan
Copy link
Member

@TomiBelan TomiBelan commented Sep 17, 2018

Ako som napísal v #122 (comment), tu je môj pokus o nový build systém.
tl;dr: lodash 4, package.json, yarn, webpack 4, babel 7, node-sass.

Ak má @kubik369 alebo @mrshu alebo nejaký náhodný okoloidúci :) čas a chuť prečítať si to a napísať mi code review, poteším sa. (Pridajte sa do Reviewers vpravo, nech viem že na to mám čakať.) Ak sa nikto nenájde, nevadí, zhruba o týždeň to mergnem.

Pôvodný dátum: 2018-07-15. Už je síce jeseň, ale meno vetvy zostalo.


This change is Reviewable

Why use Yarn instead of npm:

- Yarn is faster (although npm is better than before). See below.
  It's especially fast when there is nothing to do.
  (OTOH: This is useful if we run "yarn install" on every change, but we will
  stop doing that soon, so maybe it doesn't matter very much.)

- Yarn supports "resolutions". They might be helpful in the future to cut out
  large and unused transitive dependencies.
  (OTOH: We don't have any concrete plans to use them.)

- Yarn supports the --cwd flag. It would be convenient if package.json was
  somewhere else than the repository root.
  (OTOH: package.json is in the repository root. And buildstatic.sh uses --cwd
  for now, but it will disappear soon.)

In conclusion, we could manage with npm, but Yarn seems nicer.

Why put package.json and node_modules in the root directory (and not votrfront
or votrfront/js):

- It is convenient when you never need to use "cd" (or "--cwd=") during
  development. E.g. there are no problems with commands from the shell history
  expecting a different working directory.

- There is already precedent for votrfront-specific data in the root:
  console.py, logdb, logs, oldlogs and sessions.

- Putting package.json in the root seems to be the most common layout and other
  tools we might start using in the future might expect it.

Benchmark:

(In all tests, yarn.lock and package-lock.json already exist, and the package
data is already cached in ~/.npm and ~/.cache.)
With current package.json:
  yarn 1.9.2: 1.447s if node_modules doesn't exist, 0.499s if idle.
  npm 5.6.0:  2.611s if node_modules doesn't exist, 1.554s if idle.
With a projected package.json using webpack 4:
  yarn 1.9.2: 8.024s if node_modules doesn't exist, 0.713s if idle.
  npm 5.6.0:  7.017s if node_modules doesn't exist, 3.442s if idle.
Copy link
Collaborator

@kubik369 kubik369 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dúfam, že tá inclusion css-loader-a je aj plánovaná na využitie v budúcnosti, na trochu krajšie CSS-ká a nie len na ten jeden import v main.js ;)

Myslím si, že ten webpack config je nejaký prekomplikovaný, ale to by sa malo dať vyriešiť vyššie spomínanou prerábkou CSS a trochou pobúchania.

Inak to vyzerá vcelku v pohode :)

README.md Outdated
3. Create a virtualenv directory. A virtualenv is an isolated environment that
2. Install node.js 8+. On Ubuntu 18.04 or newer or Debian Buster or newer, use:

sudo apt install nodejs
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Silno som za vyhodenie tohoto postupu a nahradenie za NVM. Funguje na všetkých (?) distrách rovnako a dá sa kontrolovať verzia pre projekt. Rôzne distro repozitáre môžu mať veľmi ďaleké verzie.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stojí to za zamyslenie. Moja hlavná neistota je, čo s prod serverom. Rozdeľme to na dve otázky: 1. Bude prod server tiež používať nvm? 2. Ak nie, má README používať nvm napriek dev/prod rozdielu?

Bude prod server tiež používať nvm? Výhoda je omnoho väčšia kontrola nad konkrétnou verziou node. Nevýhoda je, že automatické použitie nvm je zdá sa stále dosť krkolomné - nvm často predpokladá že je sourcnuté v nejakom shelli. Viď napr. https://stackoverflow.com/a/28390848 (to je len príklad, votr nepoužíva docker). Ale vidím že existuje nvm-exec, takže možno by to išlo. Myslíš že to stojí za to?

Má README používať nvm napriek dev/prod rozdielu? FWIW, poznám tri spôsoby ako nainštalovať node: distro balíčky, nvm, a nodeenv, ktorý sa mi možno páči ešte viac (pip install nodeenv && nodeenv -p dá node do ./venv/bin). Všetky podporujeme, ale ktorý/é zdokumentovať? Ak všetky tri, README by bolo pridlhé. Ak nie produkčnú metódu, README by bolo nereprezentatívne. Čo s tým?

Jedna možnosť by bola mať v README sudo apt install nodejs a potom že ak máte staré distro alebo nechcete systémovú inštaláciu, tu je (nová) Votr wiki stránka kde sú podrobne rozpísané všetky ostatné možnosti - a tam by na prvom mieste bolo nvm, a nodesource repozitáre na poslednom. Mám to spraviť tak?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ja osobne nevidím problém v tom mať iný spôsob inštalácie node-u na dev a prod. Ak bude README pridlhé, je miesto na wiki urobiť samotný článok, ktorý bude len o inštalácií. Radšej mať dva rozdielne spôsoby, ktoré sú pohodlnejšie na oboch prostrediach, ako silou tlačiť uniformný spôsob. Development verzia sa taktiež vždy môže prispôsobiť tomu, čím by bol prípadne obmedzený server.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, dobre. Spravím článok.

That said... tuším že nerozumiem aký má nvm vlastne zmysel. Ani pre vývojárov mi nepríde nejaké super pohodlné. Na vlastnej mašine proste mám balíčky, na prod serveri tiež, a ak je cieľové publikum tohto README noví Votr developeri čo ešte nemajú nainštalovaný node, tiež by som im proste odporučil balíček a hotovo. Možnosť prepínať medzi viacerými verziami je dosť poweruserská fičúria. Čo mi uniká? Prečo by nvm mal byť "defaultný" spôsob? :)

README.md Outdated
On Ubuntu 16.04 or Debian Stretch, the default node.js is too old, but the
unofficial repo from https://nodejs.org/en/download/package-manager/ works.

3. [Install Yarn.](https://yarnpkg.com/en/docs/install)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dať iba Yarn ako odkaz (pre vzdelanie) a napísať príkaz na inštaláciu sem. Zbytočné odskakovanie od návodu do browseru kvôli pár znakom.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Súhlasil by som, ale ktorý príkaz? Yarn odporúča inštaláciu cez distro balíky. :/
Preto som tam dal len linku, nech si každý vyberie.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ja vždy robím len npm i -g yarn. Príde mi to cleaner, lebo je to naviazané na konkrétnu inštaláciu node-u (aj keď to je možno len v combe s NVM). Priznávam ale, že asi bude lepšie pre menej skúsených ľudí s týmto aby si to pozreli tam, takže beriem späť.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bohužiaľ presne npm install --global yarn explicitne neodporúčajú ;)

README.md Outdated
git clone https://github.com/fmfi-svt/votr.git
cd votr

5. Create a virtualenv directory. A virtualenv is an isolated environment that
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keď už sa to prerába, skúsil si sa pozrieť na pipenv? Je oveľa jednoduchší na používanie, dependancy management s ním je tiež oveľa jednoduchší a netreba huntovať po distro-specific python balíčkoch lebo si to samo pullne.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skúsil som to a stačilo pipenv install a nemusel som robiť nič iné, všetko sa spravilo samo.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Zaujímavý nápad. Ako sa inštaluje pipenv samotný?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Buďto cez package manager alebo priamo cez pip, tu je stránka pipenv install

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Popozeral som si to a rozhodol som sa zostať pri virtualenve.
pipenv má iný formát, spätne ale nie dopredne kompatibilný, takže je to globálne rozhodnutie. Jeden problém je, že sa ťažko inštaluje na produkčný server. Ešte nemá Ubuntu balíček, a inak odporúčajú inštalovať s --user. Asi sa nejako dá dať do /usr/local, ale meh. Druhý problém je ako ho zdokumentovať. Ich stránka (na rozdiel od yarnu) nie je dostatočne prehľadná a jednoznačná, aby som si trúfal iba na ňu linkovať. Ale proces má aspoň tri kroky: 1. nainštaluj Python a pip. 2. pip3 install --user pipenv. 3. pridaj ~/.local/bin do $PATH. Takže sa to neoplatí a virtualenv stačí.

Copy link
Member Author

@TomiBelan TomiBelan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ďakujem za review :)

Priznám sa že zatiaľ nemám žiadne konkrétne plány používať css-loader viac... ale prinajmenšom je to odteraz možné :) Dosť veľa nášho SCSS sa netýka konkrétnych stránok alebo komponentov. Ale pár častí áno (napr. log viewer, login page, anketa popup), a tie by sa dali zrefaktorovať.

Súhlasím že webpack.config.js je dosť dlhý a viac custom ako sa patrí. Trochu ma sklamalo, že som nenašiel nič podobné ako StatusFilePlugin, a že sa mi nepodarilo použiť clean-webpack-plugin. Ak máš konkrétne nápady, ako ho skrátiť, poraď.

README.md Outdated
3. Create a virtualenv directory. A virtualenv is an isolated environment that
2. Install node.js 8+. On Ubuntu 18.04 or newer or Debian Buster or newer, use:

sudo apt install nodejs
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stojí to za zamyslenie. Moja hlavná neistota je, čo s prod serverom. Rozdeľme to na dve otázky: 1. Bude prod server tiež používať nvm? 2. Ak nie, má README používať nvm napriek dev/prod rozdielu?

Bude prod server tiež používať nvm? Výhoda je omnoho väčšia kontrola nad konkrétnou verziou node. Nevýhoda je, že automatické použitie nvm je zdá sa stále dosť krkolomné - nvm často predpokladá že je sourcnuté v nejakom shelli. Viď napr. https://stackoverflow.com/a/28390848 (to je len príklad, votr nepoužíva docker). Ale vidím že existuje nvm-exec, takže možno by to išlo. Myslíš že to stojí za to?

Má README používať nvm napriek dev/prod rozdielu? FWIW, poznám tri spôsoby ako nainštalovať node: distro balíčky, nvm, a nodeenv, ktorý sa mi možno páči ešte viac (pip install nodeenv && nodeenv -p dá node do ./venv/bin). Všetky podporujeme, ale ktorý/é zdokumentovať? Ak všetky tri, README by bolo pridlhé. Ak nie produkčnú metódu, README by bolo nereprezentatívne. Čo s tým?

Jedna možnosť by bola mať v README sudo apt install nodejs a potom že ak máte staré distro alebo nechcete systémovú inštaláciu, tu je (nová) Votr wiki stránka kde sú podrobne rozpísané všetky ostatné možnosti - a tam by na prvom mieste bolo nvm, a nodesource repozitáre na poslednom. Mám to spraviť tak?

README.md Outdated
On Ubuntu 16.04 or Debian Stretch, the default node.js is too old, but the
unofficial repo from https://nodejs.org/en/download/package-manager/ works.

3. [Install Yarn.](https://yarnpkg.com/en/docs/install)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Súhlasil by som, ale ktorý príkaz? Yarn odporúča inštaláciu cez distro balíky. :/
Preto som tam dal len linku, nech si každý vyberie.

README.md Outdated
git clone https://github.com/fmfi-svt/votr.git
cd votr

5. Create a virtualenv directory. A virtualenv is an isolated environment that
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Zaujímavý nápad. Ako sa inštaluje pipenv samotný?

@kubik369
Copy link
Collaborator

Momentálne nemám nič extra konkrétne, ale môžem sa s tým skúsiť pohrať potom :) Už som ti odpísal na všetky komenty, dúfam že ti každý neposlal mail :D

@mrshu
Copy link
Contributor

mrshu commented Sep 18, 2018

Už som ti odpísal na všetky komenty, dúfam že ti každý neposlal mail :D

Ja som teda dostal, ale som za to vdacny -- celkom dost som sa naucil.

Len pre uplnost si myslim, ze je uplne spravne, ze to reviewuje @kubik369 a nie ja -- tazko si viem predstavit, ze by som mal tolkoto relevantnych komentarov :)

Copy link
Member Author

@TomiBelan TomiBelan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Veru prišlo n mailov, ale nevadí. Trik je neklikať na reply ale ísť do files tabu, tam pridať odpoveď pod nejaký thread (alebo nový komentár) a stlačiť start code review. Zvlášť neintuitívne je že aj ja musím klikať na start code review aj keď ja som ten reviewovaný.

README.md Outdated
3. Create a virtualenv directory. A virtualenv is an isolated environment that
2. Install node.js 8+. On Ubuntu 18.04 or newer or Debian Buster or newer, use:

sudo apt install nodejs
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, dobre. Spravím článok.

That said... tuším že nerozumiem aký má nvm vlastne zmysel. Ani pre vývojárov mi nepríde nejaké super pohodlné. Na vlastnej mašine proste mám balíčky, na prod serveri tiež, a ak je cieľové publikum tohto README noví Votr developeri čo ešte nemajú nainštalovaný node, tiež by som im proste odporučil balíček a hotovo. Možnosť prepínať medzi viacerými verziami je dosť poweruserská fičúria. Čo mi uniká? Prečo by nvm mal byť "defaultný" spôsob? :)

README.md Outdated
On Ubuntu 16.04 or Debian Stretch, the default node.js is too old, but the
unofficial repo from https://nodejs.org/en/download/package-manager/ works.

3. [Install Yarn.](https://yarnpkg.com/en/docs/install)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bohužiaľ presne npm install --global yarn explicitne neodporúčajú ;)

README.md Outdated
git clone https://github.com/fmfi-svt/votr.git
cd votr

5. Create a virtualenv directory. A virtualenv is an isolated environment that
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Popozeral som si to a rozhodol som sa zostať pri virtualenve.
pipenv má iný formát, spätne ale nie dopredne kompatibilný, takže je to globálne rozhodnutie. Jeden problém je, že sa ťažko inštaluje na produkčný server. Ešte nemá Ubuntu balíček, a inak odporúčajú inštalovať s --user. Asi sa nejako dá dať do /usr/local, ale meh. Druhý problém je ako ho zdokumentovať. Ich stránka (na rozdiel od yarnu) nie je dostatočne prehľadná a jednoznačná, aby som si trúfal iba na ňu linkovať. Ale proces má aspoň tri kroky: 1. nainštaluj Python a pip. 2. pip3 install --user pipenv. 3. pridaj ~/.local/bin do $PATH. Takže sa to neoplatí a virtualenv stačí.

votr.dev.js is gone for now. It will return soon.

Size of prologue + votr.min.js:
raw: 99485 -> 80933
gz:  21420 -> 19132
Loose mode can be dangerous and the size is almost equal so there's no reason
to turn it on.

Size of prologue + votr.min.js:
raw: 80933 -> 83925
gz:  19132 -> 19658
Size of jsdeps-prod:
raw: 364182 -> 357414
gz:  114864 -> 110865
(cd votrfront/static; for f in $(cat jsdeps-prod); do gzip <$f; done) | wc -c

Some of the size decrease might be because we're finally minimizing
transition.js and modal.js.
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. Not a real problem, but it
  feels less safe than wheels.
- node-sass has many more dependencies than it needs to. node_modules grows
  from 71M to 94M. But as a consolation, python-libsass doesn't strip its .so,
  so the total used disk space is pretty similar.
The plugin creates a file that says whether webpack is running, has succeeded,
or has failed. front.py reads the file and waits for the build to finish if
it's currently in progress.

This is essentially a custom substitute for webpack-dev-server. We could use
that instead, but I couldn't find a way to configure it to do what I want.
webpack-dev-server has two modes: inline mode and iframe mode. In inline mode,
it injects its own client library into every bundle. This is a problem for Votr
since we load both prologue.?.js and votr.?.js on every page. In iframe mode,
they are separated into different frames, but the server on being on top and
putting Votr in the iframe, not vice versa. When Votr navigates to another URL
or changes the page title, the user won't see. There might be a third option to
load /webpack-dev-server.js as a separate script in front.py, but it is not
documented anywhere. The documentation is lacking in general.

It's also arguable if we even want automatic refresh after every rebuild, when
AIS requests can take so long. Hot module reloading could solve that, but it's
also more difficult to configure. We can take another look later.
@TomiBelan
Copy link
Member Author

Pushol som novú verziu. Zmeny: const vo webpack.config.js; komentáre v layout.js; prepísané README s linkou na wiki stránku (ktorá ešte neexistuje).

Installing JavaScript dependencies is now an explicit step that must be done
after cloning or pulling Votr, same as Python dependencies, not something that
happens automatically.

watchstatic.py was fully replaced by webpack --watch.

By default, only development mode is built. Votr.setDebug() can still switch
between production and development if Votr was built with "yarn buildboth".
Also, make the .map file names different on every run in watch mode.
@TomiBelan
Copy link
Member Author

Yay! Ešte raz vďaka :)

@TomiBelan TomiBelan merged commit 7f5848a into master Sep 20, 2018
@TomiBelan TomiBelan deleted the summerbuild branch September 20, 2018 18:33
@TomiBelan TomiBelan mentioned this pull request Sep 20, 2018
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.

4 participants