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

Typescripten! #18

Open
wants to merge 12 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@akx
Copy link
Member

commented Jun 11, 2018

This PR:

  • adds tooling for TypeScript 2.9 3.4 (incl. Webpack 4)
  • converts the frontend source to TypeScript
  • removes some unnecessary fonts
  • removes unnecessary full imports of Lodash (since we only use a handful of functions)
  • replaces Moment with date-fns (all we used Moment for was HH:mm formatting)
  • removes babel-polyfill and whatwg-fetch, as we're targeting browsers that should support those already

Re the removals and replacements, we currently ship ~999 KB of JavaScript; it's cut to half (497 KB) with this :)

@akx akx requested review from japsu, Logima and Hilzu Jun 11, 2018

@akx akx force-pushed the typescripten branch from f693bee to 6b7911b Jun 11, 2018

@codecov-io

This comment has been minimized.

Copy link

commented Jun 11, 2018

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #18   +/-   ##
=======================================
  Coverage   88.75%   88.75%           
=======================================
  Files          10       10           
  Lines         258      258           
  Branches       16       16           
=======================================
  Hits          229      229           
  Misses         24       24           
  Partials        5        5

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 493024e...8220448. Read the comment docs.

@akx akx force-pushed the typescripten branch from 3e92aa8 to acae52b Apr 28, 2019

@akx akx force-pushed the typescripten branch from acae52b to 38db5dd Apr 28, 2019

@akx

This comment has been minimized.

Copy link
Member Author

commented Apr 28, 2019

Refreshed dependencies, etc. Could I get some eyes on this? :) 👀

EDIT: I'm also fine with a rubber-stamp review and we can see if things work in real life :)

"watch": "webpack --verbose --progress --watch",
"release": "webpack -p",
"lint": "eslint src webpack.config.js --ext .js,.jsx",
"build": "webpack --progress",

This comment has been minimized.

Copy link
@Hilzu

Hilzu Apr 29, 2019

Member

What is this third webpack build script in addition to watch and release? Usually I'd expect the build script to create a production build.

"build": "webpack --progress",
"watch": "webpack --mode=development --progress --watch",
"release": "webpack --mode=production",
"lint:ts": "tslint src/**.ts*",

This comment has been minimized.

Copy link
@Hilzu

Hilzu Apr 29, 2019

Member

These days you should use eslint instead of tslint for TypeScript linting. I just pasted an example config to #desutech couple of days ago 😄

"eslint-plugin-jsx-a11y": "^6.0.3",
"eslint-plugin-react": "^7.9.1",
"prettier": "^1.13.4"
"awesome-typescript-loader": "^5.0.0",

This comment has been minimized.

Copy link
@Hilzu

Hilzu Apr 29, 2019

Member

Is there a reason to use awesome-typescript-loader instead of just ts-loader?

"style-loader": "^0.23.1",
"url-loader": "^1.0.1",
"webpack": "^4.11.1",
"webpack-cli": "^3.0.3"
},
"prettier": {
"printWidth": 100,

This comment has been minimized.

Copy link
@Hilzu

Hilzu Apr 29, 2019

Member

😱

plugins: {
autoprefixer: {
browsers: [
'>0.25%',

This comment has been minimized.

Copy link
@Hilzu

Hilzu Apr 29, 2019

Member

This includes stuff like Safari 8 and Android Browser 4.4. For InfoTV something like >= 1% in FI, Firefox ESR, not dead, not ie 11 should be enough.

This comment has been minimized.

Copy link
@Logima

Logima Apr 29, 2019

Member

Apparently (#9) Tracon needs support for Android 4. I think Desucon runs this only in recent Chrome nowdays.

@japsu

This comment has been minimized.

Copy link
Member

commented Apr 29, 2019

onks jotain syytä olla käyttämättä ejectaamatonta react-scriptsiä 🤔


if (production) {
config.plugins.push(
plugins: [
new webpack.DefinePlugin({

This comment has been minimized.

Copy link
@Hilzu

Hilzu Apr 29, 2019

Member

There's no reason to use this plugin explicitly in modern webpack

},
],
},
resolve: {
extensions: [".js", ".jsx", ".ts", ".tsx", ".json"],
symlinks: false, // SPEED BOOST

This comment has been minimized.

Copy link
@Hilzu

Hilzu Apr 29, 2019

Member

What why? Also is the extensions list really required?

@Hilzu

This comment has been minimized.

Copy link
Member

commented Apr 29, 2019

onks jotain syytä olla käyttämättä ejectaamatonta react-scriptsiä 🤔

Totta kyllä tuokin

@japsu

japsu approved these changes Jun 10, 2019

Copy link
Member

left a comment

mun kommentit ei oo merge-blocking, voidaan kattoa noita myöhemminkin

@akx

This comment has been minimized.

Copy link
Member Author

commented Jun 10, 2019

Katotaan conin jälkeen, nyt ei taas viitti hajottaa kaikkea. :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.