These are chat archives for redfin/react-server

15th
Jul 2016
Perry Shuman
@Noirbot
Jul 15 2016 14:08
It seems your package.json doesn't follow your editorconfig rules
Perry Shuman
@Noirbot
Jul 15 2016 14:14
Or should that be an exception
Douglas Wade
@doug-wade
Jul 15 2016 17:19
interesting
I don’t think it should be an exception?
But I’ve also never gotten any warnings or anything from https://github.com/sindresorhus/atom-editorconfig
Perry Shuman
@Noirbot
Jul 15 2016 17:42
It seems to be 2-space indented
While editorconfig seems to want 4-tab intents
I'm guessing the NPM defaults to 2-space when you run npm install --save
and then Atom just runs with whatever the file already is as the default, overriding .editorconfig
Perry Shuman
@Noirbot
Jul 15 2016 17:49
Also, I can't seem to get the tests to run locally for me
Errored while running npm script 'test' in 'generator-react-server'
Error: Command failed: /bin/sh -c npm run test
sh: ava: command not found
Douglas Wade
@doug-wade
Jul 15 2016 17:50
interesting
Perry Shuman
@Noirbot
Jul 15 2016 17:51
But I've run npm install
Douglas Wade
@doug-wade
Jul 15 2016 17:51
dougwade code/react-server ‹master*› » grep ava packages/generator-react-server/package.json
    "ava": "^0.14.0",
    "test": "ava test --tap && xo && nsp check",
      "ava/no-ignored-test-files": [
it has a declared dev dependency
mmmmm
Perry Shuman
@Noirbot
Jul 15 2016 17:52
Wait, do I have to run sub-installs?
Because I just ran npm install on the root
Douglas Wade
@doug-wade
Jul 15 2016 17:52
no, you have to run an npm run bootstrap at the monorepo root
Perry Shuman
@Noirbot
Jul 15 2016 17:52
ohhh
Douglas Wade
@doug-wade
Jul 15 2016 17:52
or a lerna bootstrap
Perry Shuman
@Noirbot
Jul 15 2016 17:53
ah
I must have just missed that in the docs
Douglas Wade
@doug-wade
Jul 15 2016 17:53
is frantically adding a section to CONTRIBUTING.md
Perry Shuman
@Noirbot
Jul 15 2016 17:54
haha
Glad I can be your useful idiot
Douglas Wade
@doug-wade
Jul 15 2016 17:54
monorepos are pretty new
I hadn’t worked with them before React Server/Redfin
mostly because they have a good set of links to learn more
Perry Shuman
@Noirbot
Jul 15 2016 17:56
Nifty
Douglas Wade
@doug-wade
Jul 15 2016 18:06
redfin/react-server#349
Lemme know if there’s anything else you’d like in a Getting Started guide for contributors
Perry Shuman
@Noirbot
Jul 15 2016 18:07

Errored while running npm script 'test' in 'babel-plugin-react-server'
Error: Command failed: /bin/sh -c npm run test
module.js:339
throw err;
^

Error: Cannot find module '../modules/web.dom.iterable'

Perry Shuman
@Noirbot
Jul 15 2016 18:09
I'm trying a nuke and bootstrap
Douglas Wade
@doug-wade
Jul 15 2016 18:09
me too!
I don’t really have any inkling of what that might be
it looks like a partially downloaded dependency
what version of npm are you running, btw?
and are you on windows
Perry Shuman
@Noirbot
Jul 15 2016 18:10
OS X
10.11.5
Node v4.2.1
Perry Shuman
@Noirbot
Jul 15 2016 18:11
npm 2.14.7
Douglas Wade
@doug-wade
Jul 15 2016 18:11
def a version I’ve used w/React Server before
Perry Shuman
@Noirbot
Jul 15 2016 18:17

I'm seeing this:

Version mismatch inside "react-server-integration-tests". Depends on "react-server@../react-server" instead of "react-server@0.3.4".

This message was deleted
So now it's a different error!
Perry Shuman
@Noirbot
Jul 15 2016 18:23
It seems that gulp test is failing inside packages/react-server

/Users/perry.shuman/Code/react-server/packages/react-server/core/logging/stats.js
76:10 error Arrow function should not return assignment no-return-assign

✖ 1 problem (1 error, 0 warnings)

[13:22:52] 'eslint' errored after 2.11 s
[13:22:52] ESLintError in plugin 'gulp-eslint'
Message:
Failed with 1 error

But the actual message seems to not be percolating up through Lerna cleanly
Perry Shuman
@Noirbot
Jul 15 2016 18:30
            // The `stop` method logs the total elapsed time since
            // timer creation.
            stop: () => (mainLogger.time(token, ct = new Date - t0, opts), ct),
Douglas Wade
@doug-wade
Jul 15 2016 18:31
that’s a real error alright
you may have noticed that our ci build is broken
Perry Shuman
@Noirbot
Jul 15 2016 18:31
Haha
This may be part of that
Douglas Wade
@doug-wade
Jul 15 2016 18:32
Screen Shot 2016-07-15 at 11.31.31 AM.png
It definitely is
not sure how it made its way to master though
the CI build should have prevented it?
Perry Shuman
@Noirbot
Jul 15 2016 18:33
something, something git blame?
:P
It's some very clever code
Douglas Wade
@doug-wade
Jul 15 2016 18:37
That’s how you know its @gigabo
Perry Shuman
@Noirbot
Jul 15 2016 18:37
Haha
Douglas Wade
@doug-wade
Jul 15 2016 18:39
its also been there since 2015
4173dad6 core/logging/stats.js (Bo Borgerson 2015-07-01 16:43:27 -0700 76) stop: () => (mainLogger.time(token, ct = new Date - t0, opts), ct),
Perry Shuman
@Noirbot
Jul 15 2016 18:39
Did I just become your QA :P
Douglas Wade
@doug-wade
Jul 15 2016 18:39
so, uh...
yes please :smile:
Perry Shuman
@Noirbot
Jul 15 2016 18:39
Haha
You had a chance 2 years ago :P
Douglas Wade
@doug-wade
Jul 15 2016 18:40
:question:
Perry Shuman
@Noirbot
Jul 15 2016 18:40
I interviewed at Redfin Seattle
but got offered a back-end java dev position
Douglas Wade
@doug-wade
Jul 15 2016 18:40
and you want to be front-end-ier?
Perry Shuman
@Noirbot
Jul 15 2016 18:40
Mostly just not-Java
Douglas Wade
@doug-wade
Jul 15 2016 18:41
I hear that
Perry Shuman
@Noirbot
Jul 15 2016 18:41
Java still makes me hate life
But yea, real pretty office
The view was real tempting
Now I'll just do it for free! Yay OSS
Douglas Wade
@doug-wade
Jul 15 2016 18:42
Yay! :grinning:
looking at the listings, looks like the closest thing we have is a senior full-stack dev http://www.redfin.com/about/jobs/description/senior-software-developer-listings-orLn1fwk?from=hq
Perry Shuman
@Noirbot
Jul 15 2016 18:46
haha, not even remotely qualified for it
And team here at BV's good folks
Douglas Wade
@doug-wade
Jul 15 2016 18:47
We have talked about interviewing devs who demonstrate React Server knowledge, tho, so I’d be more than willing to refer you based on the site I pulled yesterday.
Perry Shuman
@Noirbot
Jul 15 2016 18:47
Na, it's fine
We should get coffee next time I'm in town though
Douglas Wade
@doug-wade
Jul 15 2016 18:48
:+1:
where’re you at currently?
Perry Shuman
@Noirbot
Jul 15 2016 18:48
Bazaarvoice - Austin
Douglas Wade
@doug-wade
Jul 15 2016 18:50
woah, some big name clients http://www.bazaarvoice.com/about/clients/
Perry Shuman
@Noirbot
Jul 15 2016 18:51
Haha, yea
We're ~#6 in the world in traffic, across all our clients
But all white-label stuff, so no one knows who we are
Douglas Wade
@doug-wade
Jul 15 2016 18:52
white-label?
Perry Shuman
@Noirbot
Jul 15 2016 18:53
We don't have our name on any of our content
If we do our job right, it just looks like all our code is a natural part of the client's page
But yea, we do the review services for pretty much everyone online except for Amazon
Perry Shuman
@Noirbot
Jul 15 2016 19:01
@doug-wade I'm still getting a different failure, even with that fix in
Douglas Wade
@doug-wade
Jul 15 2016 19:02
a super-helpful exit status: 1 from gulp, I’ll wager
redfin/react-server#350
Perry Shuman
@Noirbot
Jul 15 2016 19:02
Two eslint errors in packages/react-server-test-pages
Do you not run tests/lint as a push hook?

/Users/perry.shuman/Code/react-server/packages/react-server-test-pages/pages/data/delay.js
4:37 error Arrow function should not return assignment no-return-assign

/Users/perry.shuman/Code/react-server/packages/react-server-test-pages/pages/navigation/common.js
33:12 error Arrow function should not return assignment no-return-assign

Douglas Wade
@doug-wade
Jul 15 2016 19:04
we/i do not
Perry Shuman
@Noirbot
Jul 15 2016 19:04
Ah, yea, that's something we set up here that's been great
You don't get to push code up until it's passing unit tests/linting
Actually, most projects here have just hooked linting as a commit hook
Douglas Wade
@doug-wade
Jul 15 2016 19:06
I would def take a pr for that
it sounds really nice
Perry Shuman
@Noirbot
Jul 15 2016 19:06
I'd have to look into how to set the hooks in a more automated manner
We'd also likely have to work out how to get a lerna lint
Douglas Wade
@doug-wade
Jul 15 2016 19:08
I just disabled no-return-assign altogether
tis a silly rule
there is no lerna lint
I think we’d have to add a lint target to each package.json and do a lerna run lint
Perry Shuman
@Noirbot
Jul 15 2016 19:09
Doesn't seem like a bad idea
I'll look into it
Perry Shuman
@Noirbot
Jul 15 2016 19:16
Seems to be passing now
Yup, passed for me locally
Douglas Wade
@doug-wade
Jul 15 2016 19:19
I merged the fix
Perry Shuman
@Noirbot
Jul 15 2016 19:20
now it's time to rebase ALL THE THINGS
Douglas Wade
@doug-wade
Jul 15 2016 19:25
Screen Shot 2016-07-15 at 12.25.36 PM.png
how’d you guess :laughing:
Perry Shuman
@Noirbot
Jul 15 2016 19:26
Haha, I've been relinting an entire repo recently
Nothing like running eslint --fix **/*.js
So many merges
Douglas Wade
@doug-wade
Jul 15 2016 19:30
:rocket:
woah, the number of fixable eslint rules has really increased since the last time I checked http://eslint.org/docs/rules/
Perry Shuman
@Noirbot
Jul 15 2016 19:33
Yea
I love it
Douglas Wade
@doug-wade
Jul 15 2016 19:45
:feelsgood:
Screen Shot 2016-07-15 at 12.44.49 PM.png
Thanks for all your help @Noirbot!
Perry Shuman
@Noirbot
Jul 15 2016 19:45
Np!
Douglas Wade
@doug-wade
Jul 15 2016 19:45
This message was deleted
Perry Shuman
@Noirbot
Jul 15 2016 19:46
Haha, I'm working on a couple PRs
Douglas Wade
@doug-wade
Jul 15 2016 20:01
the other thing to consider instead of a push pr hook is using the eslint bot redfin/react-server#66
Perry Shuman
@Noirbot
Jul 15 2016 21:02
My inclination would be to put eslint on a commit hook
Lint issues are something you should know about ASAP
I mean, you should see them in your editor
and fix them then
but it's so fast to run, it should be trivial to run them as a pre-commit
Perry Shuman
@Noirbot
Jul 15 2016 21:15
Getting my dev environment set up on my home comp now
Perry Shuman
@Noirbot
Jul 15 2016 21:21
Haha, npm is so inefficient...
doing npm run bootstrap is pretty much bricking my Macbook Air
Perry Shuman
@Noirbot
Jul 15 2016 21:27
Yea, it's using 90% of each of my CPU cores
Douglas Wade
@doug-wade
Jul 15 2016 21:33
That sounds pretty efficient to me /s
Perry Shuman
@Noirbot
Jul 15 2016 21:49
Hmm, tests are failing locally now
Error: Command failed: /bin/sh -c npm run test
An error occurred running zombie tests [TypeError: connect ECONNREFUSED 127.0.0.1:8770]
[TypeError: connect ECONNREFUSED 127.0.0.1:8770]
{ '0': '/attributesOnRootElement', '1': [Function] }
An error occurred running zombie tests [TypeError: connect ECONNREFUSED 127.0.0.1:8770]
[TypeError: connect ECONNREFUSED 127.0.0.1:8770]
{ '0': '/cssWithAssets', '1': [Function] }
An error occurred running zombie tests [TypeError: connect ECONNREFUSED 127.0.0.1:8770]
[TypeError: connect ECONNREFUSED 127.0.0.1:8770]
{ '0': '/simpleTitle', '1': [Function] }
Douglas Wade
@doug-wade
Jul 15 2016 21:50
that ECONNREFUSED pops up from time to time
It’s a transient failure
redfin/react-server#175
The build works if you kick it off again, but its not a very good experience for contributors.
Perry Shuman
@Noirbot
Jul 15 2016 21:51
Hmm, it's happened twice in a row
Douglas Wade
@doug-wade
Jul 15 2016 21:51
!!!
that is exciting
Perry Shuman
@Noirbot
Jul 15 2016 21:51
I wonder if it's that I'm on a slow/small enough computer
Some sort of race condition?
Douglas Wade
@doug-wade
Jul 15 2016 21:52
its almost certainly some kind of race condition
i’ve never gotten it to repro locally
Perry Shuman
@Noirbot
Jul 15 2016 21:52
Time to go out and get a crappier computer :P
Yup, seems to be consistent for me
I'm... not sure how this really helps you
Douglas Wade
@doug-wade
Jul 15 2016 21:55
books a plane ticket to Austin
Perry Shuman
@Noirbot
Jul 15 2016 21:55
Haha
Douglas Wade
@doug-wade
Jul 15 2016 21:55
my bootstrap is still running
Perry Shuman
@Noirbot
Jul 15 2016 21:55
But really, maybe find a worse computer
Douglas Wade
@doug-wade
Jul 15 2016 21:55
maybe a new dep made it bad enough to repro locally
Perry Shuman
@Noirbot
Jul 15 2016 21:55
I dunno
It was working fine on my work MBP
Douglas Wade
@doug-wade
Jul 15 2016 21:55
would you mind posting your specs to the issue?
Perry Shuman
@Noirbot
Jul 15 2016 21:57
Done
Woo 1.7ghz GPU
Douglas Wade
@doug-wade
Jul 15 2016 22:00
hmmm… i’ll crank the concurrency to see if i can cause it to repro, then
Perry Shuman
@Noirbot
Jul 15 2016 22:01
If there's anything I can do, let me know
Douglas Wade
@doug-wade
Jul 15 2016 22:02
to try to get it to pass locally, you can try dumping the concurrency lerna --concurrency=2 run test
Perry Shuman
@Noirbot
Jul 15 2016 22:03
I guess with Lerna installed globally?
Douglas Wade
@doug-wade
Jul 15 2016 22:04
yeah — the contributing guide has some explanation
also lernajs.io
Perry Shuman
@Noirbot
Jul 15 2016 22:05
❯ lerna --concurrency=2 run test
Too many arguments.
?
Douglas Wade
@doug-wade
Jul 15 2016 22:05
lerna -v?
you should be running beta 24
Perry Shuman
@Noirbot
Jul 15 2016 22:05
1.1.3
Douglas Wade
@doug-wade
Jul 15 2016 22:05
npm i -g lerna@2.0.0-beta.24
Perry Shuman
@Noirbot
Jul 15 2016 22:06
oh dang
Douglas Wade
@doug-wade
Jul 15 2016 22:06
I forgot that they don’t update the latest tag to point it at the latest beta version :sheep:
Perry Shuman
@Noirbot
Jul 15 2016 22:06
❯ lerna --concurrency=2 run test
Lerna v2.0.0-beta.24
Lerna version mismatch: The current version of lerna is 2.0.0-beta.24, but the Lerna version in lerna.json is 2.0.0-beta.23. You can either run lerna init again or install lerna@2.0.0-beta.23.
irony
Douglas Wade
@doug-wade
Jul 15 2016 22:07
I merged the beta 24 upgrade ~30 min ago
so a pull from upstream/master should do you
:rocket:
Perry Shuman
@Noirbot
Jul 15 2016 22:07
haha
so fresh
Errored while running npm script 'test' in 'react-server'
Error: Command failed: /bin/sh -c npm run test
sh: gulp: command not found
That's new
Btw, if you're done with work, I don't want to keep you late on Friday
This is pretty low priority
Irony - I can't nuke to update to beta24 because nuke uses Lerna :P
Douglas Wade
@doug-wade
Jul 15 2016 22:12
:laughing:
npm run bootstrap will install it
npm run nuke doesn’t actually install deps, just delete them iirc
Perry Shuman
@Noirbot
Jul 15 2016 22:15
Yea, it's an automated rm -rf node_modules for each
Pretty much
Perry Shuman
@Noirbot
Jul 15 2016 22:25
Yea, dropped it to Concurrency 2 and it still happened
Perry Shuman
@Noirbot
Jul 15 2016 22:33
@doug-wade It only worked when I dropped it to Concurrency 1
Douglas Wade
@doug-wade
Jul 15 2016 22:58
mmmmmmmmmmmmm
interesting
Perry Shuman
@Noirbot
Jul 15 2016 23:07
Btw, is there a new release of react-server pushed to NPM?