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

Remove React from static scripts, fix Prask task accordingly #1118

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

Conversation

kubik369
Copy link
Contributor

@kubik369 kubik369 commented Jan 21, 2018

Neviem ako testnúť tú Prask úlohu na stránke, rady sú vítané.

@codecov
Copy link

codecov bot commented Jan 21, 2018

Codecov Report

Merging #1118 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1118   +/-   ##
=======================================
  Coverage   80.85%   80.85%           
=======================================
  Files         162      162           
  Lines        6969     6969           
=======================================
  Hits         5635     5635           
  Misses       1334     1334

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4f7c2e2...93912eb. Read the comment docs.

@kubik369
Copy link
Contributor Author

kubik369 commented Jan 22, 2018

Všimol som si, že React používa ešte nejaký Tip of the day. Nebudem klamať, nikdy som si nič také nevšimol na stránke a je to momentálne prázdny element na vrchu noviniek. Plánuje sa s tým niečo robiť? Ak hej, hodilo by sa urobiť v tom django_tips opäť jeden bundle, v ktorom by ten relevantný React bol, nech sa to všade neloaduje. Ak nie, tak to celé odstrániť najskôr.

@mhozza
Copy link
Member

mhozza commented Jan 22, 2018 via email

@kubik369
Copy link
Contributor Author

kubik369 commented Jan 22, 2018

To chápem, že to je dynamické a ten ReactDOM to tam má vyrenderovať, ale nič sa tam nezobrazuje, ani ako pre admina, ani v incognito, usera som neskúšal. A stále platí ten point, že by to malo mať React so sebou v bundli, keďže to je len na jednom mieste.

@mhozza
Copy link
Member

mhozza commented Jan 22, 2018 via email

@kubik369
Copy link
Contributor Author

Ok, makes sense, súhlasíš s tým, že by to malo byť bundlenuté? Ak áno, dám ti PR na django-tips.

@mhozza
Copy link
Member

mhozza commented Jan 22, 2018 via email

@kubik369
Copy link
Contributor Author

Ešte lepšie, ak by sa dalo, že vôbec ten element spolu s tým bundlom nebudú includenuté ak už nemáš tips.

{
"name": "prask_questions",
"version": "1.0.0",
"description": "compile by: ``` babel --presets es2015,react --watch src/ --out-dir lib/ ```",
Copy link
Member

Choose a reason for hiding this comment

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

preco je v description napisane nieco, co vyzera uplne odlisne ako commandy v scripts ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eh, to je pozostatok z toho čo tam Prefix/Sysel porobili, ušlo mi to.

@black3r
Copy link
Member

black3r commented Jan 22, 2018

ten prekompilovany subor z prasku sa filenamom tvari ako minifikovany, ale v skutocnosti minifikovany nie je.... nedalo by sa s tym nieco urobit? na rozdiel od toho fontu toto je 1.7MB :D

@kubik369
Copy link
Contributor Author

kubik369 commented Jan 22, 2018

@black3r My bad, fixed. Má to 203kb minifikované :)

// EDIT
Update, zabudol som pridať niektoré pluginy, už 138kb.

@@ -34,8 +34,6 @@
<![endif]-->
<!-- IE10 viewport hack for Surface/desktop Windows 8 bug -->
<script src="{% static "js/ie10-viewport-bug-workaround.js" %}"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/15.0.1/react.min.js" type="text/javascript"></script>
Copy link
Member

Choose a reason for hiding this comment

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

Toto teda nepotrebujeme uz?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nie, je to includenuté v tom js bundle-y. Čo mi pripomína, že možno by som tam mohol popchať všetky tie js veci do toho. Pozriem sa na to v stredu poobede/štvrtok.

new webpack.DefinePlugin({
'process.env.NODE_ENV': debug ? JSON.stringify('development') : JSON.stringify('production')
}),
new webpack.optimize.ModuleConcatenationPlugin(),
Copy link
Member

Choose a reason for hiding this comment

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

toto aj nieco robi, ked v babel-i nemas nakonfigurovane "modules": false ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nie, máš pravdu. Config ešte trochu refine-nem, keď sa mi podarí/niekto mi pomôže s tými django-tips a tu sa môže použiť ten istý prakticky.

Copy link
Member

Choose a reason for hiding this comment

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

Co potrebujes s tymi django-tips?

@mhozza
Copy link
Member

mhozza commented May 14, 2018

Toto je v akom stave?

@mhozza
Copy link
Member

mhozza commented Jan 28, 2019

👉 @kubik369

@mhozza
Copy link
Member

mhozza commented Jun 15, 2019

👉 @kubik369

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

Successfully merging this pull request may close these issues.

3 participants