Check for deprecated APIs

[pdehaan]

I updated the .eslintrc file to the following and also extended plugin:node/recommended and got a few deprecation notices:

➜  fxa-auth-server git:(master) ✗ grunt eslint
Running "eslint:app" (eslint) task

/Users/pdehaan/dev/github/mozilla/fxa-auth-server_moz/fxa-auth-server/lib/crypto/butil.js
   7:23  error  'Buffer()' was deprecated since v6. Use 'Buffer.alloc()' or 'Buffer.from()' (use 'https://www.npmjs.com/package/safe-buffer' for '<4.5.0') instead  node/no-deprecated-api
  32:16  error  'Buffer()' was deprecated since v6. Use 'Buffer.alloc()' or 'Buffer.from()' (use 'https://www.npmjs.com/package/safe-buffer' for '<4.5.0') instead  node/no-deprecated-api

/Users/pdehaan/dev/github/mozilla/fxa-auth-server_moz/fxa-auth-server/lib/routes/validators.js
  8:16  error  'punycode' module was deprecated since v7. Use 'https://www.npmjs.com/package/punycode' instead  node/no-deprecated-api

/Users/pdehaan/dev/github/mozilla/fxa-auth-server_moz/fxa-auth-server/test/local/crypto/butil.js
  24:20  error  'Buffer()' was deprecated since v6. Use 'Buffer.alloc()' or 'Buffer.from()' (use 'https://www.npmjs.com/package/safe-buffer' for '<4.5.0') instead  node/no-deprecated-api
  25:20  error  'Buffer()' was deprecated since v6. Use 'Buffer.alloc()' or 'Buffer.from()' (use 'https://www.npmjs.com/package/safe-buffer' for '<4.5.0') instead  node/no-deprecated-api

/Users/pdehaan/dev/github/mozilla/fxa-auth-server_moz/fxa-auth-server/test/local/senders/email.js
  10:16  error  'util._extend' was deprecated since v6. Use 'Object.assign()' instead  node/no-deprecated-api

/Users/pdehaan/dev/github/mozilla/fxa-auth-server_moz/fxa-auth-server/test/mocks.js
  13:16  error  'util._extend' was deprecated since v6. Use 'Object.assign()' instead  node/no-deprecated-api

✖ 7 problems (7 errors, 0 warnings)

Warning: Task "eslint:app" failed. Use --force to continue.

Aborted due to warnings.

extends:
  - plugin:fxa/server
  - plugin:node/recommended

plugins:
  - fxa
  - node

root: true

rules:
  node/no-missing-require: off
  node/no-unpublished-require: off
  node/shebang: off

  handle-callback-err: off
  no-process-exit: off
  strict: error

[vladikoff]

@pdehaan did you pull master , we had fixes here: 4815505

[pdehaan]

I did. Errors in question seem to be in the btutil.js lib and test files:

fxa-auth-server/lib/crypto/butil.js

Line 7 in ab7ba5a

module.exports.ONES = Buffer('ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff', 'hex')

fxa-auth-server/lib/crypto/butil.js

Line 32 in ab7ba5a

var result = Buffer(buffer1.length)

fxa-auth-server/test/local/crypto/butil.js

Lines 24 to 25 in ab7ba5a

	const b1 = Buffer('abcd', 'hex')
	const b2 = Buffer('abcd', 'hex')

[vladikoff]

@pdehaan ah different butil, yee

[deeptibaghel]

@vladikoff i think these were left in my fix :(

[pdehaan]

2 different files, 4 instances. Looks like #2338 missed a couple buried references.

[pdehaan]

@deeptibaghel No worries, I blame the guy who posted screenshots of the code in the original bug (versus showing the results using git grep or something).

➜  fxa-auth-server git:(master) ✗ git grep -n " Buffer("

lib/crypto/butil.js:7:module.exports.ONES = Buffer('ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff', 'hex')
lib/crypto/butil.js:32:  var result = Buffer(buffer1.length)
test/local/crypto/butil.js:24:        const b1 = Buffer('abcd', 'hex')
test/local/crypto/butil.js:25:        const b2 = Buffer('abcd', 'hex')

But all kidding aside, switching to eslint-plugin-node's "recommended" rules, found 2-3 other deprecated APIs that warrant consideration. They may come back to haunt you by the time you think about bumping up to node@8, or whatever.

[vladikoff]

No worries, I blame the guy who posted screenshots of the code in the original bug (versus showing the results using git grep or something).

😉

[deeptibaghel]

@vladikoff , i am able fix the deprecation of util._extend but not punycode.
Even if i install the punycode module, it keeps using the bundled one and error continues: 'punycode' module was deprecated since v7. Use 'https://www.npmjs.com/package/punycode' instead
Is there a way out ?

[vladikoff]

Use 'https://www.npmjs.com/package/punycode' instead Is there a way out ?

@pdehaan I think the only way we can fix that is maybe calling var punycode = require('../../node_modules/punycode') @pdehaan thoughts?

[vladikoff]

sorry @deeptibaghel ^, mentioned @pdehaan twice 😃

[pdehaan]

Yeah, it's a tricky problem... Using Node@6 and Node@9, it looks like I still get the bundled/deprecated punycode module:

const puny = require("punycode");
console.log(puny.version); // 2.0.0

Not sure what the best solution would be. Add an "orphaned" punycode dependency to the package.json, and then you have a fallback if they remove it in Node 10, and things would still work. Even the README at https://github.com/bestiejs/punycode.js seems to say you should do an npm install punycode --save, but locally I'm seeing that default to the bundled Node.js version instead of the downloaded version from npm.

$ node -e "console.log(require('punycode').version)" # 2.0.0
$ node -e "console.log(require('./node_modules/punycode').version)" # 2.1.0
$ node --version # v9.7.1

$ node -e "console.log(require.resolve('punycode'))" # punycode
$ node -e "console.log(require.resolve('./node_modules/punycode'))" # /Users/pdehaan/dev/tmp/node_modules/punycode/punycode.js

@jrgm: Have you/we ever seen any deprecated Node.js modules yet and have a best practice for package name collisions from npm+Node?

[vladikoff]

@mathiasbynens any ideas how to deal with the deprecation warnings above ^

[pdehaan]

Filed upstream mathiasbynens/punycode.js#79

[pdehaan]

@vladikoff If we just want the error to go away, we could just ignore that one line in ESLint (although possibly still add the dependency in package.json in the off-chance they remove the API in the future):

// eslint-disable-next-line node/no-deprecated-api
var punycode = require('punycode')

[pdehaan]

@vladikoff Lots of good conversation in mathiasbynens/punycode.js#79 and looks like they npm published under punycode.js to avoid a naming conflict, if we're still looking for workarounds for that node/no-deprecated-api error message for punycode. ¯\_(ツ)_/¯