Fails with "Path must be a string" on Node 8

marionebl commented 6 years ago

In certain conditions import-fresh fails with the error message Path must be a string. Received null on Node 8.

importFresh('global-dirs');

 Path must be a string. Received null
  module.exports.moduleId (~/node_modules/import-fresh/index.js:11:36)
  ...

Example build: travis/commitlint#L1738

Digging deeper reveals this is caused by caller-path returning a Callsite that returns null from its getFileName method: caller-path/index.js#L4

This in turn is caused by caller-callsite returning always the very first Callsite it finds by breaking out of the iteration here: caller-callsite/index.js#L13

Removing the break statement fixes the issue for me - I am not familiar with the APIs used here, so I am not very confident this is the appropriate fix though.

Steps to reproduce

n 8.9.1
git clone https://github.com/marionebl/commitlint.git
cd commitlint 
git checkout import-fresh-null-repro
npx lerna bootstrap
cd `@commitlint/core
npx ava src/library/resolve-extends.test.js

aladdin-add commented 5 years ago

same issue in eslint: https://github.com/eslint/eslint/pull/11066

aladdin-add commented 5 years ago

friendly ping @sindresorhus

sindresorhus commented 5 years ago

Can you try out v3.0.0? It improves the logic for finding the module to clear.

aladdin-add commented 5 years ago

~~seems not, I've upgrade to import-fresh@3.0.0, but getting another error: https://ci.appveyor.com/project/nzakas/eslint/builds/21226625~~

oops it's working, thanks for fixing this!

aladdin-add commented 5 years ago

@sindresorhus seems the issue could be closed!

sindresorhus commented 5 years ago

Great!

sindresorhus / import-fresh

Fails with "Path must be a string" on Node 8 #6

Steps to reproduce