Make GoKibitz buildable again

apetresc commented 5 years ago

I'm attempting to build a development version of GoKibitz and finding that it's incompatible in a few ways with NodeJS 10.16.0 LTS. This PR attempts to make the smallest number of changes needed to allow GoKibitz to run cleanly with a modern Node toolchain.

This PR is not ready for merging, I'm just opening now to hopefully get some help overcoming a few hurdles.

[x] Remove memwatch-next and heapdump dependencies memwatch-next is no longer supported and doesn't work under Node 10.16.0 (see this issue). Luckily it's not actually used for anything except a commented-out block of code. So removing the dependency doesn't break anything at all.
[x] Upgrade gulp-sass to 4.0.2 Again, the latest LTS NodeJS release isn't supported by node-sass<=4.9 as can be seen here.
[x] Provide Docker Compose setup I wanna wrap the tricky native dependencies and running a persistent mongo instance behind Docker to make it easy to run on any system without messing with nvm and junk. The NodeJS toolchain is really atrocious...

[x] Fix broken SCSS After getting a successful build, gulp default fails with:

/Users/apetresc/src/personal/gokibitz/node_modules/node-sass/lib/index.js:291
 var payload = assign(new Error(), JSON.parse(err));
Error: client/src/scss/rules/_layout.scss
Error: Illegal nesting: Only properties may be nested beneath properties.
    on line 387 of client/src/scss/rules/_layout.scss
>>            $width: 3px;

 ---^

at options.error (/Users/apetresc/src/personal/gokibitz/node_modules/node-sass/lib/index.js:291:26)

The relevant section is:

  border: {
    $width: 3px;
    color: $flat-green-sea;
    radius: 0;
//      radius: 0 0 0 $width * 2;
//      width: $width;
    width: 0;
  }

which looks like perfectly valid SCSS to me so I have no idea what's going on here. This is where I'm currently stuck.

[ ] Resolve the broken utf-8-validate module dependency At the moment, any npm build will contain a failure of the form:

/Users/apetresc/.node-gyp/10.16.0/include/node/v8config.h:327:29: note: expanded from macro 'V8_DEPRECATED'
declarator __attribute__((deprecated))
                            ^
5 warnings and 1 error generated.
make: *** [Release/obj.target/validation/src/validation.o] Error 1
gyp ERR! build error
gyp ERR! stack Error: `make` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (/Users/apetresc/.nvm/versions/node/v10.16.0/lib/node_modules/npm/node_modules/node-gyp/lib/build.js:262:23)
gyp ERR! stack     at ChildProcess.emit (events.js:198:13)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:248:12)
gyp ERR! System Darwin 18.6.0
gyp ERR! command "/Users/apetresc/.nvm/versions/node/v10.16.0/bin/node" "/Users/apetresc/.nvm/versions/node/v10.16.0/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild"
gyp ERR! cwd /Users/apetresc/src/personal/gokibitz/node_modules/utf-8-validate
gyp ERR! node -v v10.16.0
gyp ERR! node-gyp -v v3.8.0
gyp ERR! not ok

Luckily this is an optional dependency so the build continues, but I'd still like to get to the bottom of it. It happens on both my local macOS and inside the Docker container so it's likely not something wrong with my build environment but rather another deprecation/incompatibility with Node 10.16.0.

[ ] Provide instructions in README about the correct format for .amazon-ses.js, or provide a way to mock/ignore them. At the moment, user.js fails to run without it.

I expect more problems to pop up after I get past the error above, but hopefully it can all be worked through 😃

neagle commented 5 years ago

I feel equal measures of gratitude and guilt for seeing someone else put part of their lives into a chore that I have put off myself. Thanks! I'd be happy to jump in and provide assistance if you'd be willing to give me permission to contribute to your branch while it's in PR status.

Docker: I've wanted to containerize the Mongo dependency, especially, for a while. This will be a big benefit for anyone else who wants to work on this locally. If you get a moment, or when it gets to that point, could you add instructions for running the Docker project?
The SCSS error: looks like Sass no longer tolerates a variable being declared in the middle of a nested properties block. The variable can just be moved one line above to fix the new error:

    .form-control {
        background: {
            color: $flat-green-sea;
        }
        $width: 3px;
        border: {
            color: $flat-green-sea;
            radius: 0;
                          // (...and so on)

apetresc commented 5 years ago

My pleasure 😃 You should already have commit access to this branch specifically - let me know if it's not working for some reason and I can just make you a contributor on my entire fork.

Docker instructions have been added!
Thanks for the SCSS fix, that was indeed it.
I just edited the PR description with a few more TODOs; I'll just keep adding to the list as I encounter them.

neagle commented 5 years ago

The README updates are fantastic; thank you!

I tried pushing to your branch and got an error--I think you have to manually allow me to make contributions, as described here. That's only if you'd like me to help you this way--if you'd rather collaborate by just having me make suggestions here, that's fine, too.

apetresc commented 5 years ago

Yup, I had that checked since the beginning, that's why I thought it would work:

No big deal though, just added you as a collaborator on my fork so you can push to any branch including this one :)

neagle / gokibitz

Make GoKibitz buildable again #186