node modules resolver should return a fallback path for package not located by require.resolve

[thescientist13]

Type of Change

• New Feature Request
• Documentation / Website
• Improvement / Suggestion
• Bug
• Other (please clarify below)

Summary

As part of #917 , noticed that inadvertently when puppeteer was added to dependencies, all of a sudden certain packages would fail the getNodeModulesLocationForPackage utility function look up, and error out in the terminal.

{ locations: null }
Unable to look up punycode using NodeJS require.resolve.
Falling back to process.cwd()
=============

  TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined
      at validateString (internal/validators.js:124:11)
      at Object.join (path.js:1110:7)
      at walkPackageJson (file:///Users/owenbuckley/Workspace/project-evergreen/repos/greenwood/packages/cli/src/lib/walker-package-ranger.js:202:44)
      at async walkPackageJson (file:///Users/owenbuckley/Workspace/project-evergreen/repos/greenwood/packages/cli/src/lib/walker-package-ranger.js:209:11)
      at async walkPackageJson (file:///Users/owenbuckley/Workspace/project-evergreen/repos/greenwood/packages/cli/src/lib/walker-package-ranger.js:209:11)
      at async walkPackageJson (file:///Users/owenbuckley/Workspace/project-evergreen/repos/greenwood/packages/cli/src/lib/walker-package-ranger.js:209:11)
      at async walkPackageJson (file:///Users/owenbuckley/Workspace/project-evergreen/repos/greenwood/packages/cli/src/lib/walker-package-ranger.js:209:11)
      at async file:///Users/owenbuckley/Workspace/project-evergreen/repos/greenwood/packages/cli/src/plugins/resource/plugin-node-modules.js:103:13

What's interesting is that there is no change in dependecies before / after moving puppeteer into dependencies?

# before
yarn why punycode
yarn why v1.22.5
[1/4] 🤔  Why do we have the module "punycode"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "[email protected]"
info Reasons this module exists
   - "_project_#whatwg-url#tr46" depends on it
   - Hoisted from "_project_#whatwg-url#tr46#punycode"
   - Hoisted from "_project_#ajv#uri-js#punycode"
   - Hoisted from "_project_#jsdom#tough-cookie#punycode"
info Disk size without dependencies: "48KB"
info Disk size with unique dependencies: "48KB"
info Disk size with transitive dependencies: "48KB"
info Number of shared dependencies: 0
✨  Done in 0.88s.

# after
yarn why punycode
yarn why v1.22.5
[1/4] 🤔  Why do we have the module "punycode"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "[email protected]"
info Reasons this module exists
   - "_project_#whatwg-url#tr46" depends on it
   - Hoisted from "_project_#whatwg-url#tr46#punycode"
   - Hoisted from "_project_#ajv#uri-js#punycode"
   - Hoisted from "_project_#jsdom#tough-cookie#punycode"
info Disk size without dependencies: "48KB"
info Disk size with unique dependencies: "48KB"
info Disk size with transitive dependencies: "48KB"
info Number of shared dependencies: 0
✨  Done in 0.88s.

Details

Not sure why locations is null, but I suppose there's no reason we couldn't at least try and do something, like fallback to process.cwd() I suppose

if (!nodeModulesUrl) {
  console.debug(`Unable to look up ${packageName} using NodeJS require.resolve.`);
  // we could / should at least try to provide a fallback here?
}

Maybe this issue is relevant? - ajv-validator/ajv#1381

Note: somewhat related, but I think the import.meta.resolve logic is actually broken... 🙈 😬

[thescientist13]

Another related thread possibly - nodejs/node#29549