make 'debug' as dependencies

[overcache]

nodejs between v11.x - v15.5.1 has a bug: it will cache MODULE_NOT_FOUND when require a non-exits file(issue).

./debug.js try to require('debug'), if the 'debug' pkg not exits, nodejs will cache the result. Even install debug later in the same process(spawn npm install), require('debug') will still throw error

so, we should make 'debug' pacakge as dependencies.

[NE-SmallTown]

Same problem, you can execute below commands by node './index.js'. It will throw an error in node > 11.1 or node < 15.5.1

// index.js

const { spawn } = require('child_process')

async function installDep(dependencies, flags = []) {
  return new Promise((resolve, reject) => {
    const installation = spawn(
      'npm',
      [
        'install',
        '--loglevel',
        'error',
        '--no-fund',
        ...flags,
        ...dependencies,
      ],
    )

    process.on('eixt', () => installation.kill())
    process.on('SIGINT', () => installation.kill())
    process.on('uncaughtException', () => installation.kill())

    installation.on('close', code => {
      if (code === 0) {
        resolve()
      } else {
        reject()
      }
    })
  })
}

async function installPackage(package) {
  return new Promise((resolve, reject) => {
    const installation = spawn(
      'npm',
      [
        'install',
        '--loglevel',
        'error',
        package
      ],
    )

    process.on('eixt', () => installation.kill())
    process.on('SIGINT', () => installation.kill())
    process.on('uncaughtException', () => installation.kill())

    installation.on('close', code => {
      if (code === 0) {
        resolve()
      } else {
        reject()
      }
    })
  })
}

async function main() {
  console.log('install axios start')

  await installPackage('axios')

  console.log('install axios success')

  const axios = require('axios')

  console.log('require axios module: ', axios)

  console.log('install debug start')

  await installPackage('debug')

  console.log('install debug success')

  // this line will throw an error because the bug this PR described
  const res = require('debug')

  console.log('require debug module: ', res)

  console.log('finally end')
}

main()

By the way, according to the npm doc of peerDependenciesMeta, if you declare debug as peerDependenciesMeta.optional: true, that means your library will still work without the debug package(or some APIs depends on the debug some APIs are not, you can give specific usage in the doc).

But now, the follow-redirects will definitely always use the debug package, though you have added a try catch. So it's not optional, and will cause the error this PR described.

So, I would suggest to mark debug as dependencies.

[coveralls]

Coverage remained the same at 100.0% when pulling 8a755e3 on overcache:master into db771b1 on follow-redirects:master.

[RubenVerborgh]

Can't do that; see #94 #116 #117 #135 #140 #141

[NE-SmallTown]

@RubenVerborgh Thanks for your reply! I have read them before.

Just like I said above, we are using the peerDependenciesMeta field by mistake which will cause the bug described above.

We have 3 ways to fix this:

1. Don't use the try { require('...') } catch (e) {} way to detect whether the package been installed which will cause the Node.js cache bug described above.

2. Mark debug as dependencies, for Node.js@4 compatibility, we can use a lower debug version which support Node.js@4

3. Mark debug as peerDependencies , so the consumer will know that they should install it. for Node.js@4 compatibility, we can use a lower debug version which support Node.js@4

The current problem is, you mark debug as peerDependenciesMeta: optional: true, that means it's ok to use follow-redirects package if the consumer doesn't install it. But the truth is that if the consumer doesn't install it, they will get error.

[RubenVerborgh]

What I am reading is that:

• There a bug in Node 15
• You are installing new dependencies at runtime, which is a very rare situation

The most logical conclusion seems that, given the rarity of the situation, you investigate a way to clear the cache of Node 15.

• Don't use the try { require('...') } catch (e) {} way to detect whether the package been installed which will cause the Node.js cache bug described above.

Not possible; debug cannot be assumed to be installed for the reasons linked above.

• Mark debug as dependencies, for Node.js@4 compatibility, we can use a lower debug version which support Node.js@4

Idem, see linked issues.

• Mark debug as peerDependencies , so the consumer will know that they should install it. for Node.js@4 compatibility, we can use a lower debug version which support Node.js@4

See linked issues as well.

[NE-SmallTown]

@RubenVerborgh Thanks for your response!

There a bug in Node 15

Not only Node 15, but in node > 11.1 or node < 15.5.1(i.g. node 11, node 12, node 13, node 14, node 15) which covers most versions we are using

You are installing new dependencies at runtime, which is a very rare situation

Almost every CLI tool(CRA, vue-cli, next.js or any CLI tool inside company) will install dependencies at runtime. I think it's not a rare situation, instead, it's a common situation today. On the contrary, compare to the libraries or projects inside company, installing dependencies statically is a rare situation because it's for a start kit project or open-source library.

Not possible; debug cannot be assumed to be installed for the reasons linked above.

I was not saying we can assume "the debug be assumed to be installed", I just said the try catch way has a bug. That means, we should choose another way to to that.

I'm sorry I forget to give the way to check whether the package be installed, there are 2 ways to fix the try catch bug if you don't think you misuse peerDependenciesMeta: optional: true :

1. Use npm ls rather than try catch require

2. Move the try catch require into the function body like module.exports = function debug() { try catch require... }

[RubenVerborgh]

You are installing new dependencies at runtime, which is a very rare situation

Almost every CLI tool(CRA, vue-cli, next.js or any CLI tool inside company) will install dependencies at runtime.

Yeah, but those tools don't run at application runtime. You're the first running into this issue.

2\. Move the `try catch require` into the function body like `module.exports = function debug() { try catch require... }`

I could accept a PR for that.

[NE-SmallTown]

Fixed in PR: #149