tape-testing / tape

tap-producing test harness for node and browsers
MIT License
5.77k stars 307 forks source link

Correct esm problem on windows #559

Closed David-Desmaisons closed 2 years ago

David-Desmaisons commented 3 years ago

When using tape on windows in a esm module (using "type": "module" in the project package.json), this exception is raised:

(node:7484) UnhandledPromiseRejectionWarning: Error [ERR_UNSUPPORTED_ESM_URL_SCHEME]: Only file and data URLs are
supported by the default ESM loader
    at Loader.defaultResolve [as _resolve] (internal/modules/esm/resolve.js:698:11)
    at Loader.resolve (internal/modules/esm/loader.js:97:40)
    at Loader.getModuleJob (internal/modules/esm/loader.js:243:28)
    at Loader.import (internal/modules/esm/loader.js:178:28)
    at importModuleDynamically (internal/modules/cjs/loader.js:1080:27)
    at exports.importModuleDynamicallyCallback (internal/process/esm_loader.js:37:14)
    at importOrRequire (C:\...\tests\node_modules\tape\bin\import-or-require.js:11:9)       
    at C:\...\tests\node_modules\tape\bin\tape:84:14
    at Array.reduce (<anonymous>)
    at importFiles (C:\...\tests\node_modules\tape\bin\tape:81:30)

In turns out that making explicit the file protocol in the importOrRequire resolves this problem.

David-Desmaisons commented 3 years ago

I just made a test with the --require option and it seems that it is not calling the importOrRequire function: image

Is it correct or I am missing something?

ljharb commented 3 years ago

Sorry, you’re right; i meant --import=packageName

ljharb commented 3 years ago

Also, on what node version are you seeing this issue? And what exact tape command are you running?

codecov-commenter commented 3 years ago

Codecov Report

Merging #559 (9179395) into master (5a5a574) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #559   +/-   ##
=======================================
  Coverage   96.49%   96.49%           
=======================================
  Files           4        4           
  Lines         627      627           
  Branches      145      145           
=======================================
  Hits          605      605           
  Misses         22       22           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5a5a574...9179395. Read the comment docs.

David-Desmaisons commented 3 years ago

Here is a minimal repo for reproduction using tape **/*.spec.js command: https://github.com/David-Desmaisons/tape-tests

I am testing with node 12.

ljharb commented 3 years ago

You can’t pass unquoted globs to tools tho - your shell will expand them. Try putting quotes around the glob.

If that doesn’t fix it, I’ll take a look at your repro repo shortly.

David-Desmaisons commented 3 years ago

I changed to:

  "scripts": {
    "test": "tape \"**/*.spec.js\""
  },

With the same results: it works on ubuntu but breaks on windows.

ljharb commented 3 years ago

interesting, ok. it's also notable you're not using --import or --require, which means this line of code shouldn't be executed. I don't have Windows to be able to test, unfortunately.

ljharb commented 3 years ago

Indeed, your repo passes fine on my Mac.

David-Desmaisons commented 3 years ago

The path are constructed line 59 of tape.js file

image

It is then used using the import sintax but as on windows it is a windows path it is not compatible with an url:

image

Adding "file:\\" before the path resolve the windows case without breaking linux build.

ljharb commented 2 years ago

@David-Desmaisons thank you so much for bringing the problem to my attention; I'm going to go with the solution in #571, and longer term, we'll look into running CI tests on windows to prevent this kind of problem in the future.