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

Patch gift so it doesn't break things. #93

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

froboy
Copy link
Contributor

@froboy froboy commented Nov 17, 2017

This should resolve #92, but may very well break other things...

This PR:

  • updates npm
  • so that we can run npx
  • so that we can run patch-package
  • so that we can unbreak gift (a dependency of gulp-gh-pages)
  • so that we can deploy without problems.

Since we update npm, this also changes from a shrinkwrap-based lockfile to the new npm-native lock.

I make no guarantees about this not breaking other things and I don't have a project to test it on, but it should work.

@aCyborg
Copy link
Contributor

aCyborg commented Nov 20, 2017

I will test this on my project.

@aCyborg
Copy link
Contributor

aCyborg commented Nov 20, 2017

So when I work with this I get the following error:

npm ERR! Linux 4.2.0-42-generic
npm ERR! argv "/usr/bin/node" "/usr/bin/npm" "update"
npm ERR! node v0.12.18
npm ERR! npm  v2.15.11
npm ERR! code ELIFECYCLE

npm ERR! [email protected] install: `node install.js`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the [email protected] install script 'node install.js'.
npm ERR! This is most likely a problem with the phantomjs-prebuilt package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR!     node install.js
npm ERR! You can get information on how to open an issue for this project with:
npm ERR!     npm bugs phantomjs-prebuilt
npm ERR! Or if that isn't available, you can get their info via:
npm ERR! 
npm ERR!     npm owner ls phantomjs-prebuilt
npm ERR! There is likely additional logging output above.

npm ERR! Please include the following file with any support request:
npm ERR!     /var/www/mount-sinai-mv.local/styleguide/npm-debug.log

Additionally, I am getting a bunch of "Warnings" regarding the version of specific node modules that are 'wanted' vs. installed like:
npm WARN engine [email protected]: wanted: {"node":">=4"} (current: {"node":"0.12.18","npm":"2.15.11"})

I also get this syntax error, though it doesn't actually error out on this:

/var/www/mount-sinai-mv.local/styleguide/node_modules/butler/node_modules/gulp-accessibility/node_modules/access-sniff/node_modules/phantomjs-prebuilt/node_modules/request/node_modules/hawk/node_modules/boom/lib/index.js:5
const Hoek = require('hoek');
^^^^^
SyntaxError: Use of const in strict mode.
    at exports.runInThisContext (vm.js:73:16)
    at Module._compile (module.js:443:25)
    at Object.Module._extensions..js (module.js:478:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at Object.<anonymous> (/var/www/mount-sinai-mv.local/styleguide/node_modules/butler/node_modules/gulp-accessibility/node_modules/access-sniff/node_modules/phantomjs-prebuilt/node_modules/request/node_modules/hawk/lib/index.js:5:33)
    at Module._compile (module.js:460:26)
    at Object.Module._extensions..js (module.js:478:10)

Debug Log:
npm-debug.log

@froboy
Copy link
Contributor Author

froboy commented Nov 20, 2017

@aCyborg what do you get for node --version and npm --version on your project?

(if the lines in the error above are any indication, you need to check the version recommendations I put in the readme and update accordingly) jk i didn't put those in the readme here...

Here's what's required:
Version Requirements

  • Node.js - v7.10.1 ( always < v8.x as this has breaking conflicts)
  • npm - >= v5.2

@aCyborg
Copy link
Contributor

aCyborg commented Nov 20, 2017

ah got yah, I'll take a look at the readme. My versions are:
Node: v0.12.18
NPM: 2.15.11

@froboy
Copy link
Contributor Author

froboy commented Nov 20, 2017

@aCyborg just updated my comment above... I hadn't added them to the readme here, Just on the project repo.

@liberatr
Copy link

@froboy Node v7 "is not long-term support" so is that just the version that was out when this patch was created? I am experiencing a "gift" issue on a project and looking for a solution.

With homebrew it's easy to install node v6 or v8, but not v7.

@froboy
Copy link
Contributor Author

froboy commented Jan 30, 2018

I've installed node v7 with nvm or n I think... but I'm not sure we will be able to use Node 8 until #64 is resolved (maybe)?

Copy link
Contributor

@labbydev labbydev left a comment

Choose a reason for hiding this comment

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

@froboy doesn't the script to patch it need to be added to the package.json file?

"scripts": {
    "prepare": "patch-package"
  },

@froboy
Copy link
Contributor Author

froboy commented Feb 19, 2018

@labbydev yea, sorry. It's been a while since I touched this. With npm I think the script needs to be "prepare": "npx patch-package", and with yarn it's just patch-package... I'm not sure how to deal with that (I've moved to yarn for a few projects already... it might be easier in the long run to just decide to ditch npm altogether)

@froboy
Copy link
Contributor Author

froboy commented Mar 29, 2018

This should probably be refactored to use this approach fourkitchens/emulsify-gulp#74

@biz123
Copy link
Contributor

biz123 commented Sep 21, 2018

I got this working using the 4 kitchens approach in the branch palos-health-circlci-fixes but that also has other customizations just for Palos Health, so a clean branch is needed just with the switch from gulp-gh-pages to the gh-pages package and then updating the deploy task. I renamed it deploy-gh-pages because we are looking at possibly having tasks to deploy to other branches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

spress-deploy fails with npm run deploy
5 participants