Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

require() errors not propagated upwards to calling function #1256

Closed
mrousavy opened this issue Apr 18, 2024 · 2 comments
Closed

require() errors not propagated upwards to calling function #1256

mrousavy opened this issue Apr 18, 2024 · 2 comments

Comments

@mrousavy
Copy link
Contributor

mrousavy commented Apr 18, 2024

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
When adding an optional dependency (require(...) in a try/catch block), you need to put the try/catch exactly at the same location as the require(..).
The try/catch cannot be somewhere else where the require(..) is called through a function.

This breaks:

const getModule = () => require('bla') // <-- error thrown here already

let module = null
try {
  module = getModule()
} catch (e) {
  console.log('module not available.') // <-- error is NOT caught here
}

This works:

const getModule = () => {
  try {
    return require('bla')
  } catch (e) {
    console.log('module not available.') // <-- error caught safely
    return null
  }
}

let module = getModule()

The error thrown in example 1:

 BUNDLE  ./index.js 

error: Error: Unable to resolve module ihgbewui from /Users/mrousavy/Projects/react-native-vision-camera/package/src/dependencies/WorkletsProxy.ts: ihgbewui could not be found within the project or in these directories:
  ../node_modules
  ../../node_modules
  13 |
  14 | const testProxy = createModuleProxy('ughire', () => {
> 15 |   return require('ihgbewui')
     |                   ^
  16 | })
  17 |
    at ModuleResolver.resolveDependency (/Users/mrousavy/Projects/react-native-vision-camera/package/example/node_modules/metro/src/node-haste/DependencyGraph/ModuleResolution.js:139:15)
    at DependencyGraph.resolveDependency (/Users/mrousavy/Projects/react-native-vision-camera/package/example/node_modules/metro/src/node-haste/DependencyGraph.js:277:43)
    at Object.resolve (/Users/mrousavy/Projects/react-native-vision-camera/package/example/node_modules/metro/src/lib/transformHelpers.js:169:21)
    at Graph._resolveDependencies (/Users/mrousavy/Projects/react-native-vision-camera/package/example/node_modules/metro/src/DeltaBundler/Graph.js:473:35)
    at Graph._processModule (/Users/mrousavy/Projects/react-native-vision-camera/package/example/node_modules/metro/src/DeltaBundler/Graph.js:261:38)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async Graph._traverseDependenciesForSingleFile (/Users/mrousavy/Projects/react-native-vision-camera/package/example/node_modules/metro/src/DeltaBundler/Graph.js:249:5)
    at async Graph.traverseDependencies (/Users/mrousavy/Projects/react-native-vision-camera/package/example/node_modules/metro/src/DeltaBundler/Graph.js:157:9)
    at async DeltaCalculator._getChangedDependencies (/Users/mrousavy/Projects/react-native-vision-camera/package/example/node_modules/metro/src/DeltaBundler/DeltaCalculator.js:281:42)
    at async DeltaCalculator.getDelta (/Users/mrousavy/Projects/react-native-vision-camera/package/example/node_modules/metro/src/DeltaBundler/DeltaCalculator.js:112:16)

What is the expected behavior?

I expect it to propagate the error upwards so I can catch it later.

Please provide your exact Metro configuration and mention your Metro, node, yarn/npm version and operating system.

  • npm: 10.5.2
  • dependencies:
    "devDependencies": {
      "@babel/core": "^7.22.10",
      "@babel/preset-env": "^7.22.10",
      "@babel/runtime": "^7.23.9",
      "@react-native/eslint-config": "^0.72.2",
      "@react-native/metro-config": "^0.72.9",
      "@react-native/typescript-config": "^0.73.0",
      "@types/react": "^18.2.48",
      "@types/react-native-vector-icons": "^6.4.13",
      "@types/react-native-video": "^5.0.15",
      "babel-plugin-module-resolver": "^5.0.0",
      "eslint": "^8.46.0",
      "eslint-plugin-prettier": "^5.0.0",
      "metro-react-native-babel-preset": "^0.77.0",
      "prettier": "^3.2.4",
      "typescript": "^5.1.6"
    }
  • metro.config.js:
    const { getDefaultConfig, mergeConfig } = require('@react-native/metro-config')
    const path = require('path')
    const escape = require('escape-string-regexp')
    const exclusionList = require('metro-config/src/defaults/exclusionList')
    const pak = require('../package.json')
    
    const root = path.resolve(__dirname, '..')
    const modules = Object.keys({ ...pak.peerDependencies })
    
    /**
     * Metro configuration
     * https://facebook.github.io/metro/docs/configuration
     *
     * @type {import('metro-config').MetroConfig}
     */
    const config = {
      watchFolders: [root],
    
      // We need to make sure that only one version is loaded for peerDependencies
      // So we block them at the root, and alias them to the versions in example's node_modules
      resolver: {
        blacklistRE: exclusionList(
          modules.map(
            (m) =>
              new RegExp(`^${escape(path.join(root, 'node_modules', m))}\\/.*$`)
          )
        ),
    
        extraNodeModules: modules.reduce((acc, name) => {
          acc[name] = path.join(__dirname, 'node_modules', name)
          return acc
        }, {}),
      },
    
      transformer: {
        getTransformOptions: async () => ({
          transform: {
            experimentalImportSupport: false,
            inlineRequires: true,
          },
        }),
      },
    }
    
    module.exports = mergeConfig(getDefaultConfig(__dirname), config)
@robhogan
Copy link
Contributor

robhogan commented Apr 18, 2024

Yes, this is a limitation of "optional dependencies" at the moment.

The default expectation is that dependencies are non-optional and should throw at build time when missing, which is what you see in the first example. In general that's typically more likely to be a mistyped import or unintentionally missing npm dependency and failing fast is preferred.

Optional dependencies are a special case - when we can determine by single module static analysis (currently, this needs to be a try/catch directly around the require - which could be more sophisticated but with static analysis could never cover everything) that the missing dependency is handled at runtime, we infer that's your intention and effectively build the bundle with a hole in it.

One option is to try/catch locally (to let Metro know you're handing the missing dependency) and rethrow (to handle it elsewhere at runtime). Would that work for you?

@mrousavy
Copy link
Contributor Author

gotcha, thanks! yea, try / catch & rethrow is what I'm doing right now that works :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants