rollup / rollup-plugin-commonjs

This module has moved and is now available at @rollup/plugin-commonjs / https://github.com/rollup/plugins/blob/master/packages/commonjs
MIT License
502 stars 126 forks source link

Handling try-catch requires #355

Closed guybedford closed 4 years ago

guybedford commented 6 years ago

A common pattern in Node.js for detecting if a package is present / optional dependencies is (in eg https://unpkg.com/debug@3.2.6/src/node.js):

var supportsColor;
try {
  supportsColor = require('supports-color');
}
catch (e) {
}

It should be possible to support this with the following rule:

  1. When a require is within a try block, treat it as an "optional require".
  2. Attempt to resolve optional requires using this.resolve. If it does, built it in as a dependency.
  3. If it does not resolve, do not list it as a dependency at all, and instead replace the require statement with an error that throws:
var supportsColor;
try {
  var e = new Error('Cannot find module \'supports-color\'.');
  e.code = "MODULE_NOT_FOUND";
  throw e;
}
catch (e) {
}

Will work on a PR shortly.

evocateur commented 5 years ago

Another instance of a try-catch require that gets busted by Rollup (hoists require('worker_threads') to the top of the scope, no longer wrapped in try/catch, breaks everybody who doesn't pass --experimental-worker to node):

const threadId = (function getId () {
  try {
    const workerThreads = require('worker_threads')

    /// if we are in main thread, this is set to `0`
    return workerThreads.threadId
  } catch (e) {
    // worker_threads are not available, fallback to 0
    return 0
  }
})()
evocateur commented 5 years ago

In any case, if you're aware of what module is being yoinked out of its cozy try/catch, you can workaround this by adding the module ID to the ignore config:

  commonjs({
    ignore: ['worker_threads'],
  })
shellscape commented 4 years ago

Hey folks (this is a canned reply, but we mean it!). Thanks to everyone who participated in this issue. We're getting ready to move this plugin to a new home at https://github.com/rollup/plugins, and we have to do some spring cleaning of the issues to make that happen. We're going to close this one, but it doesn't mean that it's not still valid.

We've got some time yet before the move while we resolve pending Pull Requests, so if this issue is still relevant, please @ me and I'll make sure it gets reopened and transferred to the new repo. :beer: