Refactor to Modern Typescript + TSLint + Prettier #34

damassi · 2018-09-17T05:50:37Z

This PR lightly refactors the app into a modern idiomatic typescript form, and adds some nice project hygiene tools that are often seen in the JS ecosystem.

Overview of changes

Removed namespaces in favor of formal import statements so that things are more modular and scalable:

import { foo } from './bar'

Added TSLint for linting during development
Added Prettier for automatic code formatting
Added @types/.. definitions which are automatically read in by VSCode; no need to use /// refs
Added an extensions.json and settings.json file for VSCode and included TSLint and Prettier so that as code is written it's checked and formatted on the fly after save
Added lint-staged, a library which connects to git precommit and push hooks so that errors aren't checked into the repo. On commit it will run TSLint and Prettier, and on push it will run the typechecker to ensure code is valid
Added a webpack.config file for bundling up typescript for web. Typescript by default doesn't know how to bundle code that uses ES6 modules and so webpack is the tool that packages it all up for distribution.
Fixed sourcemap support

In terms of actual code changes, most of the diff is Prettier being run over the sources and TSLint being run with --fix (which automatically fixes common issues; let to const, for example, if the variable isn't reassigned). Actual code changes only relate to swapping namespaces for imports and adding some tooling.

In terms of deploying changes, the same process applies as before -- npm run dev will watch the project and output a file in the /docs folder. I've added an additional npm run build:dist command that will minify output, but didn't connect that to anything, it's just there if you think the bundle size is too large and want to bring it down for consumers. Webpack handles minification and sourcemaps, which is determined based on whether the NODE_ENV is set to development or production.

To take PR for a spin:

git checkout -b damassi-refactor-typescript master
git pull https://github.com/damassi/gtr-cof.git refactor-typescript
npm install 
npm run dev

I've added further comments inline.

damassi · 2018-09-17T05:55:40Z

package.json

+    "precommit": "lint-staged",
+    "prepush": "npm run type-check",
+    "prettier": "npm run prettier",
+    "type-check": "tsc --noEmit --pretty",


Since webpack now handles compilation output, tsc is only used for type-checking. Note that webpack reads in the tsconfig.json file for its settings.

damassi · 2018-09-17T05:56:06Z

package.json

@@ -3,18 +3,50 @@
  "version": "1.0.0",
  "description": "gtr-cof",
  "scripts": {
+    "clean": "rm -rf docs/scripts",
+    "build:dist": "NODE_ENV=production webpack",


This will produce a production-ready, minified JS bundle

damassi · 2018-09-17T05:58:11Z

src/chord-interval-module.ts

+import * as d3 from "d3"
+import * as events from "./events-module"


In order to not change the source-code too much, I used wildcard imports throughout. If wanted tho, could refactor this and pluck out individual exports like

import { chordIntervalChange } from './events-module'

damassi · 2018-09-17T05:59:29Z

src/chord-interval-module.ts

+
+    const svg = d3.select("#modes")
+    const intervals = svg.append("g").attr("transform", "translate(0, 240)")


TSLint swapped out all lets that aren't reassigned to const. In modern JS code this is used for safety since a const, if attempting to be reassigned, will throw an error.

damassi · 2018-09-17T06:02:29Z

src/tuning-module.ts

-        { tuning: "GDAE", dots: guitarDots, description: "Violin" },
-        { tuning: "CGDA", dots: guitarDots, description: "Viola" },
-    ]
+const tunings: TuningInfo[] = [


Another example of an automatic TSLint --fix change: TuningInfo[] is preferred over Array<TuningInfo>.

damassi · 2018-09-17T06:03:41Z

tsconfig.json

+        "downlevelIteration": true,
+        "lib": ["dom", "es2015", "es2016", "es2017"],
+        "module": "es2015",
+        "moduleResolution": "node",


This makes interfacing with libraries from NPM more natural.

damassi · 2018-09-17T06:08:20Z

tsconfig.json

I added a typings folder for dealing with webmidi types, which I'm not sure why are not being picked up from @types. I'm sure this can be refactored out (perhaps by adding @types/webmidi to the types config item in `tsconfig) but for now was added to satisfy the type checker.

(EDIT, answered my own question -- needed to be added to types array)

damassi · 2018-09-17T06:10:28Z

tslint.json

+    "class-name": false,
+    "interface-name": false,
+    "member-access": false,
+    "object-literal-sort-keys": false


These rules were added so that I didn't have to refactor code unnecessarily. I would recommend at the minimum turning on only-arrow-functions -- function brings with it this and all of its baggage, and so arrow functions tend to simplify and make code more functional. The other values should also be turned on, but will require light code modifications.

damassi · 2018-09-17T06:11:04Z

typings/webmidi.d.ts

+// Project: http://www.w3.org/TR/webmidi/
+// Definitions by: six a <https://github.com/lostfictions>
+// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped
+interface Navigator {


These d.ts files were copied over from @types to satisfy the type-checker. They can be removed at some point.

(No longer needed; see #34 (comment))

damassi · 2018-09-17T06:11:49Z

webpack.config.js

+const { NODE_ENV = "development" } = process.env
+
+module.exports = {
+    mode: NODE_ENV,


mode can be development or production. If production -- read from NODE_ENV -- code will minify

damassi · 2018-09-17T06:12:32Z

webpack.config.js

+    mode: NODE_ENV,
+    devtool: "sourcemap",
+    entry: "./src/index.ts",
+    stats: "errors-only",


If your curious about stats around bundle sizes and what files are being compiled, comment out this line. I set it to errors-only so that it's less verbose.

damassi · 2018-09-17T06:13:43Z

webpack.config.js

+        rules: [
+            {
+                test: /\.tsx?$/,
+                use: "ts-loader",


Loaders are plugins that compile different file types. ts-loader is what handles typescript transpilation, which is then passed back to webpack which stitches everything together into a browser-friendly single file.

damassi · 2018-09-17T06:14:45Z

webpack.config.js

+        ],
+    },
+    resolve: {
+        extensions: [".ts"],


If you were ever to introduce React into the project -- .tsx files -- then this would need to be amended to include that extension.

damassi · 2018-09-17T06:26:16Z

.vscode/settings.json

+  "editor.rulers": [120],
+  "editor.tabSize": 4,
+  "editor.formatOnSave": true,


This hooks into Prettier

damassi · 2018-09-17T06:27:20Z

package.json

+    "prepush": "npm run type-check",
+    "prettier": "npm run prettier",
+    "type-check": "tsc --noEmit --pretty",
+    "watch": "NODE_ENV=development webpack --watch"


Start webpack in development mode and watch for changes, automatically outputting to /docs (per webpack.config.js setting)

mikehadlow · 2018-09-18T09:01:10Z

Hi Christopher, thanks for the work you've put into this, I really appreciate it. I'm pretty new to Typescript and I've really only used it for this project, so there's a lot for me to digest here. I'm also, unfortunately, super busy at the moment, so it may take me a while to look at it, I hope that's OK.

damassi · 2018-09-18T17:02:52Z

Np! Just want to note that the actual typescript changes are very minimal (remove namespaces) and that this PR mostly has to do with general application dev workflows (linting, codeformatting, making sure that errors aren't checked in to git, configuring VSCode a bit, etc), which improves productivity. The diff is mostly Prettier (which works like Golang's go-fmt, or Scala's ScalaFmt) which you can play with on the playground. Taking out Prettier's formatting, the change count would be fairly small, but Prettier is pretty standard these days since it eliminates so much futzing with code.

But yeah, no rush!

https://prettier.io/playground/

Convert to modern TS and lint

0a307c0

damassi force-pushed the refactor-typescript branch from 7379eee to d0457f2 Compare September 17, 2018 05:51

damassi commented Sep 17, 2018

View reviewed changes

damassi force-pushed the refactor-typescript branch from d0457f2 to 7e165e1 Compare September 17, 2018 06:06

damassi commented Sep 17, 2018

View reviewed changes

Add webpack for browser module bundling

d7db2fb

damassi force-pushed the refactor-typescript branch from 7e165e1 to d7db2fb Compare September 17, 2018 06:24

damassi commented Sep 17, 2018

View reviewed changes

.vscode/settings.json

"editor.rulers": [120],

"editor.tabSize": 4,

"editor.formatOnSave": true,

Copy link

Contributor Author

damassi Sep 17, 2018

Choose a reason for hiding this comment

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

This hooks into Prettier

damassi commented Sep 17, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor to Modern Typescript + TSLint + Prettier #34

Refactor to Modern Typescript + TSLint + Prettier #34

damassi commented Sep 17, 2018 •

edited

Loading

damassi Sep 17, 2018

damassi Sep 17, 2018

damassi Sep 17, 2018

damassi Sep 17, 2018

damassi Sep 17, 2018

damassi Sep 17, 2018

damassi Sep 17, 2018 •

edited

Loading

damassi Sep 17, 2018

damassi Sep 17, 2018

damassi Sep 17, 2018

damassi Sep 17, 2018

damassi Sep 17, 2018

damassi Sep 17, 2018

damassi Sep 17, 2018

damassi Sep 17, 2018

damassi Sep 17, 2018

mikehadlow commented Sep 18, 2018

damassi commented Sep 18, 2018 •

edited

Loading

		import * as d3 from "d3"
		import * as events from "./events-module"


		const svg = d3.select("#modes")
		const intervals = svg.append("g").attr("transform", "translate(0, 240)")

Refactor to Modern Typescript + TSLint + Prettier #34

Are you sure you want to change the base?

Refactor to Modern Typescript + TSLint + Prettier #34

Conversation

damassi commented Sep 17, 2018 • edited Loading

Overview of changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

damassi Sep 17, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikehadlow commented Sep 18, 2018

damassi commented Sep 18, 2018 • edited Loading

damassi commented Sep 17, 2018 •

edited

Loading

damassi Sep 17, 2018 •

edited

Loading

damassi commented Sep 18, 2018 •

edited

Loading