artsy / team-navigator

An internal HR product for Artsy's team
https://team.artsy.net
MIT License
62 stars 19 forks source link

v2 #68

Closed orta closed 7 years ago

orta commented 7 years ago

Uses MuralJS - fixes #57 #66

damassi commented 7 years ago

Really awesome work on the new framework @craigspaeth đź‘Ź

Only comment (and I'm even not sure if this is possible with the current setup) is that we should try to avoid relative import paths ../../../foo which are brittle and something of a long-standing-yet-largely-ignored anti-pattern in the node community and instead setup something like https://github.com/tleunen/babel-plugin-module-resolver. I know Atom provides IDE support for this, and I imagine someone has built something for VSCode? By setting up https://www.npmjs.com/package/babel-node at the root of the project (along with a .babelrc) you should be able to enable proper paths (and use ES6+) all the way thorough the pipeline as well as support client/server path isomorphism out of the box.

craigspaeth commented 7 years ago

Thanks @damassi! I totally agree—some developer experience things the Node community fights against confuse me like relative imports and hot reloading (server-side). FWIW I did try to encourage changing NODE_PATH to aknowledge the lib folder as a first-class citizen like node_modules, but I'd support going further than that too. I'm sure Browserify supports a variety ways to achieve non-relative requires as I remember substack wrote a whole doc on Github listing the ways to do it (can't seem to find that now).

damassi commented 7 years ago

It's surprising how few know about this magical NODE_PATH variable!

orta commented 7 years ago

Fixes #29 #42 #47 #63 #57 #11

orta commented 7 years ago

Fixes #54 #53