aseemk / requireDir

Node.js helper to require() directories.
MIT License
484 stars 59 forks source link

Broken on node v8 #45

Closed raineorshine closed 7 years ago

raineorshine commented 7 years ago

There appears to be a change in require in node v8 that breaks require-dir:

C:\workspace\software-engineering\angular-mean>ncu
C:\Zimmermann\npm-config\node_modules\npm-check-updates\node_modules\require-dir\index.js:97
            if (!require.extensions.hasOwnProperty(ext)) {
                                    ^

TypeError: require.extensions.hasOwnProperty is not a function
    at requireDir (C:\Zimmermann\npm-config\node_modules\npm-check-updates\node_modules\require-dir\index.js:97:37)
    at Object.<anonymous> (C:\Zimmermann\npm-config\node_modules\npm-check-updates\lib\versionmanager.js:8:23)
    at Module._compile (module.js:569:30)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:503:32)
    at tryModuleLoad (module.js:466:12)
    at Function.Module._load (module.js:458:3)
    at Module.require (module.js:513:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (C:\Zimmermann\npm-config\node_modules\npm-check-updates\lib\npm-check-updates.js:14:10)
    at Module._compile (module.js:569:30)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:503:32)
    at tryModuleLoad (module.js:466:12)
    at Function.Module._load (module.js:458:3)
    at Module.require (module.js:513:17)

Reported at: https://github.com/tjunnone/npm-check-updates/issues/355

e1111o commented 7 years ago

The same thing happened to me. Thank you for your quick fix.

hienpham2tiki commented 7 years ago

I just sent a PR #48 for this issue

Eli-Goldberg commented 7 years ago

Hi, Thanks for the quick fix ! When can this fix be published to npm?

krnlde commented 7 years ago

Also, regarding the use of require.extensions, which is deprecated since v0.10.6:

https://nodejs.org/api/globals.html#globals_require_extensions

What alternatives do we have?

for (ext of Object.keys(require.extensions)) {
   // ...
}

would at least eliminate the use of .hasOwnProperty().

mathiasbynens commented 7 years ago

Patch: #46

krnlde commented 7 years ago

Mind my implementation with Object.keys, which renders the use of hasOwnProperty obsolete. #49

davidhellmann commented 7 years ago

Hm when goes this fix live? :(

aseemk commented 7 years ago

Sorry folks, I've changed jobs and don't have time to maintain this library right now. I'm happy to add people as contributors — please just respect semver and get some amount of consensus before making major changes. =)

Anyone interested in maintaining? cc @scragg0x and @krnlde — thank you both for the recent PRs!

aseemk commented 7 years ago

I went ahead and merged @scragg0x's PR #46. I still need to publish to npm; doing so soon.

aseemk commented 7 years ago

Okay, this is live on npm as 0.3.2.

Thank you again @scragg0x and others for reporting and fixing, and for your patience!

The offer to help maintain still stands for anyone interested.

aseemk commented 7 years ago

Btw, at my previous company (@FiftyThree), we stopped using this package (requireDir) in the last year or so, because we moved to ES6 modules (import ..., via TypeScript in our case). ES6 modules don't support dynamic import paths, so dynamically reading the contents of a directory was no longer possible.

But! It looks like there's a proposal to support dynamic import:

https://github.com/tc39/proposal-dynamic-import

And it looks like that's being tracked or polyfilled by TypeScript, Babel, etc.:

https://github.com/Microsoft/TypeScript/issues/12364 https://github.com/airbnb/babel-plugin-dynamic-import-node

So maybe it'll be time for an importDir at some point. =)

simonua commented 7 years ago

@aseemk et al., thank you very much! I verified in my consumer that this is now working properly with both node 7.10.0 and 8.0.0 (previously failed with require-dir@0.3.1).

stephenlacy commented 7 years ago

I would be interested in contributing/maintaining, we use this project heavily at my company.

aseemk commented 7 years ago

@stevelacy: Done! Thank you in advance for your help. =)

faisalraza commented 6 years ago

I installed nvm (Node Version Manager) and downgraded node version from 8.8.1 to stable 6.11.0 and it resolved the issue.

nvm install 6.11.0

fidelman commented 6 years ago

Added next code on my machine instead of requireDir('./gulp/tasks')

const normalizedPath = require('path').join(__dirname, './gulp/tasks');

require('fs').readdirSync(normalizedPath).forEach(function(file) {
  require('./gulp/tasks/' + file);
});

Looks like working

yocontra commented 6 years ago

@faisalraza @fidelman This is a solved problem (for a long time) - just update require-dir to the latest version.

fidelman commented 6 years ago

@contra I tried about 7h ago, and I got

The engine "node" is incompatible with this module. Expected version "7.8.0".
error An unexpected error occurred: "Found incompatible module".

Node v8.10.0

yocontra commented 6 years ago

@fidelman What are you using to install it? This module has "node": "*" for engines so it will work with any node version.

Try clearing your cache and making sure you're getting the right version.