rxaviers / cldr-data-npm

Npm module for Unicode CLDR JSON data
MIT License
42 stars 27 forks source link

cldr-data installation fails when request is present in package.json #33

Open perazzi opened 8 years ago

perazzi commented 8 years ago

For some weird reason, the deployment of my app started failing today. Nothing changed in the code. I'm trying to deploy the same commit that was deployed yesterday, but I'm getting now errors on cldr-data npm's installation.

It happens when my package.json has both request and cldr-data as dependencies.

The error changes from time to time, but mostly, it says

> cldr-data@28.0.4 install /Users/andreperazzi/Documents/IBM/Projects/Web Innovations/ifundit/ifundit/node_modules/cldr-data
> node install.js

module.js:338
    throw err;
          ^
Error: Cannot find module 'boom'
    at Function.Module._resolveFilename (module.js:336:15)
    at Function.Module._load (module.js:278:25)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at Object.<anonymous> (/Users/andreperazzi/Documents/IBM/Projects/Web Innovations/ifundit/ifundit/node_modules/request/node_modules/hawk/lib/index.js:3:33)
    at Module._compile (module.js:460:26)
    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)`

I found this closed issue: https://github.com/rxaviers/cldr-data-npm/issues/11

It is true for me that running npm install request before npm install solves the issue, however, I'm deploying my code to IBM Bluemix, and I can't manually run npm install. The module must be inside package.json

I have no idea why this started happening today, with no code changes.

Also, I was able to reproduce the issue by just creating an empty app and trying npm install (run npm cache clear first:

package.json:

{
  "name": "tst",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "dependencies": {
    "cldr-data": "^28.0.2",
    "request": "^2.40.0"
  }
}
rxaviers commented 8 years ago

I could not reproduce your bug. I'm running npm 3.3.6, node 5, on linux. I've created a brand new directory, created a package.json with the contents above and ran npm install, it worked just fine.

Are you experiencing this problem on your machine as well as in your deploy at bluemix? What's your setup? It's indeed a weird error.

PS: FWIW, cldr-data uses cldr-data-downloader, which depends on request": "2.72.0".

perazzi commented 8 years ago

This problem was happening with everyone in my team. I was able to fix the problem by removing the ^ character before one of the module versions. If I leave the character on both, the installation fails.

rxaviers commented 8 years ago

Please which OS, npm, and node versions are you using?

perazzi commented 8 years ago

Locally we're all using: Mac OS El Capitan NPM 2.11.3 Node v0.12.7

On Bluemix, we're using:

"engines": {
    "node": "0.12"
  }

I'm not sure which is the NPM version on Bmx, trying to figure out...

mike14511 commented 8 years ago

Also experiencing this problem. FWIW, Bluemix will default to npm version: 2.14.12.

rxaviers commented 8 years ago

I havent had time myself to debug this, but I'm definitely open for bug fixes.

jasonkarns commented 8 years ago

I'm able to repro but only with npm2. Install succeeds with npm3.

https://gist.githubusercontent.com/anonymous/f402c9c14e0e7def2610bae3bc0568c6/raw/338afa189d3cdab3a03366b5a260277799f809e4/-

jasonkarns commented 8 years ago

I suspect what's happening is:

This explains why it only occurs with npm2 (since npm3 has rewritten most of the deduping and builds a full dep tree before running); and it explains why the error message varies slightly each time, since it is nondeterministic which (if any) of request's deps aren't fully installed at the time cldr-data's install script runs.

Unfortunately, the postinstall hook doesn't run at a meaningfully different time, so doing the node install at postinstall time doesn't solve this issue. Though I'm curious why this step isn't part of prepublish. It's pretty typical that any non-architecture-dependent tasks be run at prepublish time. From npm's docs:

If you need to perform operations on your package before it is used, in a way that is not dependent on the operating system or architecture of the target system, use a prepublish script. This includes tasks such as:

  • Compiling CoffeeScript source code into JavaScript.
  • Creating minified versions of JavaScript source code.
  • Fetching remote resources that your package will use.

(emphasis mine)

And even stronger language from the docs:

Don't use install. Use a .gyp file for compilation, and prepublish for anything else. You should almost never have to explicitly set a preinstall or install script. If you are doing this, please consider if there is another option. The only valid use of install or preinstall scripts is for compilation which must be done on the target architecture.

Downloading the CLDR data is precisely what prepublish is intended for and would certainly solve this issue. Why is the node install script run at install time and not prepublish?

jasonkarns commented 8 years ago

Here are the relevant issues on npm for this bug (it is, after all, npm's bug)

https://github.com/npm/npm/issues/8850 https://github.com/npm/npm/issues/4134 https://github.com/npm/npm/issues/4096 https://github.com/npm/npm/issues/8152

The fix is not being backported to npm2. The workarounds as mentioned in https://github.com/npm/npm/issues/4134 and https://github.com/npm/npm/issues/8850 are:

puzrin commented 7 years ago

I confirm, had this problem with npm2, and no issues with npm3.

Probably almost all moved to node 6+ for es2015+, and this issue can be closed as "wontfix / outdated"