resolve 1.7.0 fails to find bootstrap's package.json

[mgedmin]

We're using bootstrap-loader (pinned at version 2.0.0-beta.22) in our webpack configuration to load bootstrap (pinned at version 4.0.0-alpha.6). bootstrap-loader depends on resolve. We were not pinning resolve's version. Our build worked with resolve 1.6.0 but fails with 1.7.0 with this curious error message:

ERROR in ./~/bootstrap-loader/lib/bootstrap.loader.js!./~/bootstrap-loader/no-op.js
Module build failed: Error: Cannot find module '/builds/.../node_modules/bootstrap/package.json/package.json'
    at Function.Module._resolveFilename (module.js:325:15)
    at Function.Module._load (module.js:276:25)
    at Module.require (module.js:353:17)
    at require (internal/module.js:12:17)
    at exports.default (/builds/.../node_modules/bootstrap-loader/lib/utils/checkBootstrapVersion.js:8:20)
    at Object.module.exports.pitch (/builds/.../node_modules/bootstrap-loader/lib/bootstrap.loader.js:159:65)
 @ ./~/bootstrap-loader/loader.js 2:17-61
 @ multi ./app/App.jsx bootstrap-loader

Note that it's looking for node_modules/bootstrap/package.json/package.json instead of node_modules/bootstrap/package.json, which is an existing file.

[ljharb]

Thanks for the report - I'll look into this and try to publish a patch today.

[ljharb]

OK, so - https://unpkg.com/bootstrap-loader@2.0.0-beta.22/loader.js is using nonstandard require syntax, which means the webpack loader is kicking in (instead of standard node resolution algorithms, which this package supports).

Do you know where that path (/builds/.../node_modules/bootstrap/package.json/package.json) is coming from, or where in bootstrap-loader this resolve call is coming from? It would also help if you're able to figure out exactly what string is being passed into resolve, and what it's wrongly returning :-) (It's kind of looking like it's in https://unpkg.com/bootstrap-loader@2.0.0-beta.22/lib/utils/resolveModule.js)

[ljharb]

Also, have you tried with a non-beta bootstrap-loader version?

[ljharb]

@JVanAartsen thanks; do you know what the value of bootstrapPath is there?

[JVanAartsen]

@ljharb whoops, deleted that cuz i convinced myself that was the wrong file. For anyone else following: https://github.com/shakacode/bootstrap-loader/blob/master/src/utils/checkBootstrapVersion.js

bootstrapPath was '... node_modules/bootstrap/package.json'

[ljharb]

Are the three dots actually in there, or is that your $PWD?

[ljharb]

@JVanAartsen so if bootstrapPath already has package.json in it, then the path.join is what's adding the extra part, not resolve. What calls checkBootstrapVersion?

[ljharb]

It looks like https://github.com/shakacode/bootstrap-loader/blob/master/src/bootstrap.loader.js#L132-L134 - what's the bootstrapPath set to in your config?

[JVanAartsen]

@ljharb yes, three dots are my pwd. not passing bootstrapPath option, so it is undefined. I'm guessing the issue stems from here https://github.com/shakacode/bootstrap-loader/blob/master/src/bootstrap.loader.js#L110 . the resolveModule call returns .../node_modules/bootstrap/package.json. I'm not familiar with that function's behavior though, maybe thats as intended.

[ljharb]

In that case, it looks like the change might be in https://github.com/shakacode/bootstrap-loader/blob/master/src/utils/resolveModule.js#L14 - I suspect that #146 might be related; @lbogdan, can you weigh in?

[krainboltgreene]

Okay, so for clarity I have a machine that works and a machine that fails with the above message. I have tracked it down to this:

exports.default = function (module) {
  try {
    var resolvedPath = undefined;
    _resolve2.default.sync(module, {
      packageFilter: function packageFilter(pkg, pathToModule) {
        resolvedPath = pathToModule;
        return pkg;
      }
    });
    return resolvedPath;
  } catch (error) {
    return false;
  }
};

This code, on the working machine, only returns a single package.json file:

{ _args: [ [Array] ],
     _from: 'bootstrap-sass@3.3.6',
     _id: 'bootstrap-sass@3.3.6',
     _inBundle: false,
     _integrity: 'sha1-NjsNMA6GjT5wE0wadCuxcohET9E=',
     _location: '/bootstrap-sass',
     _phantomChildren: {},
     _requested:
      { type: 'version',
        registry: true,
        raw: 'bootstrap-sass@3.3.6',
        name: 'bootstrap-sass',
        escapedName: 'bootstrap-sass',
        rawSpec: '3.3.6',
        saveSpec: null,
        fetchSpec: '3.3.6' },
     _requiredBy: [ '/' ],
     _resolved: 'https://registry.npmjs.org/bootstrap-sass/-/bootstrap-sass-3.3.6.tgz',
     _spec: '3.3.6',
     _where: '/Users/kurtis.rainboltgreene/Code/GOAT/sneakers/client',
     bugs: { url: 'https://github.com/twbs/bootstrap-sass/issues' },
     contributors: [ [Object], [Object], [Object], [Object] ],
     description: 'bootstrap-sass is a Sass-powered version of Bootstrap 3, ready to drop right into your Sass powered applications.',
     devDependencies: { ejs: '~2.3', mincer: '~1.3', 'node-sass': '~3.4.2' },
     homepage: 'https://github.com/twbs/bootstrap-sass#readme',
     keywords: [ 'bootstrap', 'sass', 'css' ],
     license: 'MIT',
     main: 'assets/javascripts/bootstrap.js',
     name: 'bootstrap-sass',
     repository: { type: 'git', url: 'git://github.com/twbs/bootstrap-sass.git' },
     version: '3.3.6' },

However, on the broken machine it returns two:

{ _args: [ [Array] ],
     _from: 'bootstrap-sass@3.3.6',
     _id: 'bootstrap-sass@3.3.6',
     _inBundle: false,
     _integrity: 'sha1-NjsNMA6GjT5wE0wadCuxcohET9E=',
     _location: '/bootstrap-sass',
     _phantomChildren: {},
     _requested: 
      { type: 'version',
        registry: true,
        raw: 'bootstrap-sass@3.3.6',
        name: 'bootstrap-sass',
        escapedName: 'bootstrap-sass',
        rawSpec: '3.3.6',
        saveSpec: null,
        fetchSpec: '3.3.6' },
     _requiredBy: [ '/' ],
     _resolved: 'https://registry.npmjs.org/bootstrap-sass/-/bootstrap-sass-3.3.6.tgz',
     _spec: '3.3.6',
     _where: '/Users/kurtis.rainboltgreene/code/sneakers/client',
     bugs: { url: 'https://github.com/twbs/bootstrap-sass/issues' },
     contributors: [ [Object], [Object], [Object], [Object] ],
     description: 'bootstrap-sass is a Sass-powered version of Bootstrap 3, ready to drop right into your Sass powered applications.',
     devDependencies: { ejs: '~2.3', mincer: '~1.3', 'node-sass': '~3.4.2' },
     homepage: 'https://github.com/twbs/bootstrap-sass#readme',
     keywords: [ 'bootstrap', 'sass', 'css' ],
     license: 'MIT',
     main: 'assets/javascripts/bootstrap.js',
     name: 'bootstrap-sass',
     repository: { type: 'git', url: 'git://github.com/twbs/bootstrap-sass.git' },
     version: '3.3.6' }

The weird thing is the "path" for this package (pkgFile) is considered to be:

/User/kurtis.rainboltgreene/code/sneakers/client/node_modules/bootstrap-sass/package.json

[krainboltgreene]

For what it's worth, I am not using the beta branch of that library. I am using the stable 1.0.0 branch.

[JVanAartsen]

@ljharb @lbogdan https://github.com/browserify/resolve/blob/bdf1210d96f3e06c00fad051917950eb5238ab05/lib/sync.js#L89 should this instead be pkg = opts.packageFilter(pkg, dir) maybe?

[krainboltgreene]

Okay, so I figured out why the good machine works. It's still package-locked to 1.4.0, where as the broken machine is on 1.7.0.

[ljharb]

@krainboltgreene presumably if you lock it to v1.6 it will continue to work. If there's any chance you could help do a git bisect between https://github.com/browserify/resolve/tree/v1.6.0 and https://github.com/browserify/resolve/tree/v1.7.0, that would help me out :-)

@JVanAartsen it's done twice in lib/async: one two the same way as the line you linked in lib/sync :-/

However, in v1.6, it's passed with x as the second arg - where x is the directory.

It's entirely possible that changing x to pkgfile is both the correct bugfix and also a breaking change, and if so I'll revert just that part (and restore the pkgfile behavior in v2)

Could either of you help me provide a repro case (a failing test, perhaps in a PR) for this? That would help me ensure I'm fixing the issue you're seeing.

[krainboltgreene]

I might have time for this tonight.

[krainboltgreene]

For what it's worth though, I think you're correct that this is a double edged sword: Both a bug fix and a breaking change.

[ljharb]

For history: Apparently the commit that made the original change in async failed to do so for sync, and was done as a bugfix in v1.1.3 in February 2015.

[krainboltgreene]

I know this can be difficult, but that change really should have warranted a major version bump.

[udi-d]

For a quick workaround add resolve@1.5.0 to your package.json to lock the version, or upgrade your Node server to latest, seems to fix the issue

[krainboltgreene]

We locked ours to 1.6.0 and things work great.

[ljharb]

@krainboltgreene any chance you could either provide a repro case, or check v1.7.0 with the suggested change, so I can verify that will fix it?

[lbogdan]

@JVanAartsen @ljharb packageFilter is sent to resolve.sync() (and to resolve(), too) as an option, and the doc (readme) says that it should be

opts.packageFilter(pkg, pkgfile) - transform the parsed package.json contents before looking at the "main" field

so I don't really see how changing it to opts.packageFilter(pkg, dir) would solve the issue.

[ljharb]

@lbogdan the problem is that the docs for sync were either wrong or missing previously, and since packageFilter was added, it’s always passed dir as the second argument - i agree that it should have passed pkgfile, but now that people are relying on the wrong behavior, it probably needs to stay broken in the v1 line.

[rouralberto]

Agree with @ljharb, v1 should stay with that behaviour.

[ljharb]

v1.7.1 has been released with this fix. 70850d2b5b710136a51a903cbcf63a73f22295f9 restores the "breaking" change and also makes packageFilter take three arguments, so that it's easier to migrate the code that needs "dir" (and will be released in v2).

[mgedmin]

Thank you very much! 1.7.1 works great!

[tybro0103]

I just fixed a problem of mine by pinning resolve to 1.6.0... seems possibly related to this. TBH, I don't understand what's going on, but hoping maybe someone here will.

My projects runs on both server and client, with browserify building a bundle for each. Client build is fine (at least in dev where is the only difference is just watchify), but the server/node build is not. I didn't notice the problem in dev because dev doesn't run the code through browserfiy for the server. Node bundle builds without errors, but upon execution I get the error:

TypeError: Cannot read property 'numeral' of undefined

on line

if (typeof window !== 'undefined' && this.numeral && this.numeral.language)

which is in numeral.js (here) and gets included into my app via require('numeral/languages/de') from another package in node_modules.

If you're thinking this doesn't sound related, I'd be inclined to agree, considering the build succeeded and all. But I found this by doing a diff on output of npm ls on an older, working docker image against my current build (my builds do npm install from a clean slate), and manually pinning things that had changed 1-by-1 until pinning resolve@1.6.0 fixed things. I've undone and redone this pinning a couple of times in isolation, and am sure it was the fix. 1.7.1 is the version which causes my error. Haven't tried 1.7.0.

I'm now inspired to make my build process more robust, but any insight here would be appreciated.

My browserify (^14.4.0) options for node target are:

        entries: ['./scripts/run.js'],
        extensions: ['.js', '.jsx'],
        debug: true,
        browserField: false,
        bare: true,
        commondir: false,
        builtins: false,
        insertGlobalVars: {
            process: () => undefined,
        },

[ljharb]

@tybro0103 it may be the same cause as this issue, but it's definitely a different problem - would you mind filing a new issue?

[tybro0103]

@ljharb done https://github.com/browserify/resolve/issues/159