search

jspm / github

Github Location Service
16 stars 43 forks source link

Installing bootstrap with jspm 0.16.53 and node 10 throws "TypeError [ERR_INVALID_CALLBACK]: Callback must be a function" #125

Closed gavinaiken closed 5 years ago

gavinaiken commented 5 years ago

A node deprecation https://nodejs.org/api/deprecations.html#deprecations_dep0013_fs_asynchronous_function_without_callback has become an error in node v10 and causes a jspm install of bootstrap to fail.

env:

$ node --version
v10.10.0
$ jspm --version
0.16.53

steps to reproduce (some command output removed for brevity):

$ mkdir test
$ cd test
$ npm init -y
Wrote to test/package.json:
...
$ jspm init -y
...
ok   Loader files downloaded successfully
$ jspm install bootstrap=github:twbs/bootstrap -o "{jspmPackage: true}"

warn Running jspm globally, it is advisable to locally install jspm via npm install jspm --save-dev.
     Looking up github:twbs/bootstrap
     Updating registry cache...
     Downloading github:twbs/bootstrap@4.1.3
     Looking up npm:jquery
ok   Installed npm:jquery (3.3.1)
     Looking up github:HubSpot/tether
ok   Installed github:HubSpot/tether@^1.1.1 (1.4.4)

warn Error on download for github:twbs/bootstrap
     TypeError [ERR_INVALID_CALLBACK]: Callback must be a function
         at makeCallback (fs.js:137:11)
         at Object.mkdir (fs.js:713:16)
         at /usr/local/lib/node_modules/jspm/node_modules/jspm-github/github.js:588:29
         at tryCatch (/usr/local/lib/node_modules/jspm/node_modules/rsvp/dist/rsvp.js:525:12)
         at invokeCallback (/usr/local/lib/node_modules/jspm/node_modules/rsvp/dist/rsvp.js:538:13)
         at publish (/usr/local/lib/node_modules/jspm/node_modules/rsvp/dist/rsvp.js:508:7)
         at flush (/usr/local/lib/node_modules/jspm/node_modules/rsvp/dist/rsvp.js:2415:5)
         at process._tickCallback (internal/process/next_tick.js:61:11)

err  Error downloading github:twbs/bootstrap.

warn Installation changes not saved.

Doing the same thing with node v8.11.3 succeeds:

$ jspm install bootstrap=github:twbs/bootstrap -o "{jspmPackage: true}"
     Looking up github:twbs/bootstrap
     Updating registry cache...
     Downloading github:twbs/bootstrap@4.1.3
     Looking up npm:jquery
     Looking up github:HubSpot/tether
     Downloading github:HubSpot/tether@1.4.4
(node:14508) [DEP0013] DeprecationWarning: Calling an asynchronous function without callback is deprecated.

warn Main entry point not found for github:twbs/bootstrap@4.1.3.
     Adjust this property in the package.json or with an override, setting "main": false if this is the intention.

ok   Installed github:HubSpot/tether@^1.1.1 (1.4.4)
ok   Installed bootstrap as github:twbs/bootstrap@^4.1.3 (4.1.3)
     Clearing configuration for github:twbs/bootstrap@3.3.4
     Removing package files for github:twbs/bootstrap@3.3.4
ok   Install tree has no forks.

ok   Install complete.
gavinaiken commented 5 years ago

Using the sync version of fs.mkdir resolves the issue, although this is maybe not the best approach:

diff --git a/github.js b/github.js
index a4ae063..1a80deb 100644
--- a/github.js
+++ b/github.js
@@ -585,9 +585,7 @@ GithubLocation.prototype = {
           .on('finish', function() {
             return clearDir(tmpDir)
             .then(function() {
-              return asp(fs.mkdir(tmpDir));
-            })
-            .then(function() {
+              fs.mkdirSync(tmpDir);
               return new Promise(function(resolve, reject) {
                 var files = [];
                 yauzl.open(tmpFile, function(err, zipFile) {
gavinaiken commented 5 years ago

Or using the experimental fs.promises api is another possibility:

diff --git a/github.js-orig b/github.js
index a4ae063..6a26c15 100644
--- a/github.js-orig
+++ b/github.js
@@ -585,6 +585,9 @@ GithubLocation.prototype = {
           .on('finish', function() {
             return clearDir(tmpDir)
             .then(function() {
+              if (fs.promises) {
+                return fs.promises.mkdir(tmpDir);
+              }
               return asp(fs.mkdir(tmpDir));
             })
             .then(function() {

or of course bluebird promisify, or manually wrapping fs.mkdir with a promise, or other solutions!

gavinaiken commented 5 years ago

Happy to put up a PR if any of those approaches sounds good.

guybedford commented 5 years ago

I've updated jspm-github - this should be working now! Please let me know if you're still seeing it after upgrading.

gavinaiken commented 5 years ago

Thanks, that fixed it!