search

sindresorhus / normalize-url

Normalize a URL
MIT License
837 stars 123 forks source link

Fix CI on Node.js 12 and 14 #139

Closed midgleyc closed 3 years ago

midgleyc commented 3 years ago

Fix the failing CI tests on Node 12 and 14.

It looks like the error message for some invalid URLs has changed in 16. I've changed it to a regex that matches both.

xo has a (few levels deep) peer dependency on typescript. On npm < 7, this isn't installed automatically on npm install, and you get the error message seen in the CI:

Error: Failed to load parser '/home/runner/work/normalize-url/normalize-url/node_modules/@typescript-eslint/parser/dist/index.js' declared in 'BaseConfig': Cannot find module 'typescript'
Require stack:
- /home/runner/work/normalize-url/normalize-url/node_modules/@typescript-eslint/typescript-estree/dist/parser.js
- /home/runner/work/normalize-url/normalize-url/node_modules/@typescript-eslint/typescript-estree/dist/index.js
- /home/runner/work/normalize-url/normalize-url/node_modules/@typescript-eslint/parser/dist/parser.js
- /home/runner/work/normalize-url/normalize-url/node_modules/@typescript-eslint/parser/dist/index.js
- /home/runner/work/normalize-url/normalize-url/node_modules/@eslint/eslintrc/lib/config-array-factory.js
- /home/runner/work/normalize-url/normalize-url/node_modules/@eslint/eslintrc/lib/index.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:815:15)
    at Function.Module._load (internal/modules/cjs/loader.js:667:27)
    at Module.require (internal/modules/cjs/loader.js:887:19)
    at require (internal/modules/cjs/helpers.js:74:18)
    at Object.<anonymous> (/home/runner/work/normalize-url/normalize-url/node_modules/@typescript-eslint/typescript-estree/dist/parser.js:31:25)
    at Module._compile (internal/modules/cjs/loader.js:999:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1027:10)
    at Module.load (internal/modules/cjs/loader.js:863:32)
    at Function.Module._load (internal/modules/cjs/loader.js:708:14)
    at Module.require (internal/modules/cjs/loader.js:887:19)
sindresorhus commented 3 years ago

Yeah, it's https://github.com/xojs/xo/issues/555.

sindresorhus commented 3 years ago

CI seems to still be failing.

midgleyc commented 3 years ago

Yes, now it's the code coverage, which only runs on 14. nyc ava is returning 0 for everything. Not sure whether nyc supports ESM. Could try c8 instead?

midgleyc commented 3 years ago

Running c8 locally, it generates the lcov.info file, so I think that should work.

sindresorhus commented 3 years ago

Thanks :)