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

Update NPM dependencies #84

Merged
merged 5 commits into from
Jun 4, 2018

Conversation

wolfpilot
Copy link
Contributor

@wolfpilot wolfpilot commented May 2, 2018

This PR will:

  • Fix a bunch of JS linting errors that were previously unchecked, including a bit of refactoring of the getDataPath() function (nunjucks-extensions.js).
  • Update all dependencies to their latest versions, details below.

Notes:

  • gulp-sass v3.5+ throws deprecation warnings about .css files not being importable in the future unless using custom CSS importers. Reason's that gulp-sass is indirectly based on libsass which introduced this change around end of March 2018 (link1, link2).
  • gulp-stylelint now requires stylelint as a separate dependency.
  • uglify-js v3 simplified their API which made it entirely non-backwards compatible with uglify-js@2, so I had to update some code according to the new API.

As far as I'm aware, all gulp tasks work correctly, I tested all of them throughout and at the end of the update to make sure everything's ok.

Now... can we get that dependencies badge already? 😄

@branneman
Copy link
Contributor

Works fine on the current LTS (v8), but not on Stable (v10). I'm not sure if we can fix that, it'll probably start working soon. What say you?

An npm i on Node.js v10.2.0 fails:

/Users/bvandermeer/.node-gyp/10.2.0/include/node/node.h:53:10: fatal error: 'core.h' file not found
#include "core.h"  // NOLINT(build/include_order)
         ^~~~~~~~
1 error generated.
make: *** [Release/obj.target/binding/src/binding.o] Error 1
gyp ERR! build error
gyp ERR! stack Error: `make` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (/Users/bvandermeer/Source/frontend-boilerplate-razvan/node_modules/node-gyp/lib/build.js:258:23)
gyp ERR! stack     at ChildProcess.emit (events.js:182:13)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:237:12)
gyp ERR! System Darwin 17.5.0
gyp ERR! command "/Users/bvandermeer/.nvm/versions/node/v10.2.0/bin/node" "/Users/bvandermeer/Source/frontend-boilerplate-razvan/node_modules/node-gyp/bin/node-gyp.js" "rebuild" "--verbose" "--libsass_ext=" "--libsass_cflags=" "--libsass_ldflags=" "--libsass_library="
gyp ERR! cwd /Users/bvandermeer/Source/frontend-boilerplate-razvan/node_modules/node-sass
gyp ERR! node -v v10.2.0
gyp ERR! node-gyp -v v3.6.2
gyp ERR! not ok
Build failed with error code: 1
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: [email protected] (node_modules/fsevents):
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: [email protected] install: `node install`
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: Exit status 1

An npm run dev on Node.js v10.0.0 fails:

$ npm run dev

> [email protected] dev /.../frontend-boilerplate-wolfpilot
> node node_modules/gulp/bin/gulp.js dev

[13:49:12] Requiring external module babel-core/register
gulp[41987]: ../src/node_contextify.cc:631:static void node::contextify::ContextifyScript::New(const FunctionCallbackInfo<v8::Value> &): Assertion `args[1]->IsString()' failed.
 1: node::Abort() [/Users/bvandermeer/.nvm/versions/node/v10.0.0/bin/node]
 2: node::InternalCallbackScope::~InternalCallbackScope() [/Users/bvandermeer/.nvm/versions/node/v10.0.0/bin/node]
 3: node::contextify::ContextifyScript::New(v8::FunctionCallbackInfo<v8::Value> const&) [/Users/bvandermeer/.nvm/versions/node/v10.0.0/bin/node]
 4: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo*) [/Users/bvandermeer/.nvm/versions/node/v10.0.0/bin/node]
 5: v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<true>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) [/Users/bvandermeer/.nvm/versions/node/v10.0.0/bin/node]
 6: v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) [/Users/bvandermeer/.nvm/versions/node/v10.0.0/bin/node]
 7: 0x3cfa85c8427d

@wolfpilot
Copy link
Contributor Author

@branneman First issue looks related to a broken Node v10.2.0 release - link. A hotfix is in the works and will be pushed soon, so you can wait and try again after that :)

Not sure about v10.0.0 not compiling. Have you tried deleting node_modules and running install again?

@branneman
Copy link
Contributor

Yea, I've tried rm -rf node_modules, npm rebuild and npm i --build-from-source

@peeke
Copy link
Contributor

peeke commented Jun 1, 2018

Updated my existing projects modules and fixed the errors with your updated uglifyjs code. Instead of normalize-scss we used normalize-libsass. (Not using stylelint). Will test a clean copy pasta of your PR into the project right now.

Copy link

@Thomas-Machielsen Thomas-Machielsen left a comment

Choose a reason for hiding this comment

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

Am a bit late to the party but I read through all the updated packages their npm pages, only uglyfi-js is not backwards compatible. Don't think that's much of an issue. I also ran the linter and there are no errors, so I'd approve this pull request.

@peeke
Copy link
Contributor

peeke commented Jun 4, 2018

Tested this in an existing project with Node v8.11.1 and v10.0.0, works fine 👍
Also, ran ncu -u again, there are 4 updates since the last changes. They seem to cause no problems, but maybe let's get this one merged first :)

@branneman branneman merged commit bdbdaaa into mirabeau-nl:master Jun 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants