parse-url at version 6 doesn't work on Safari

[edenhermelin]

Hi,
since version 6 of the module it seems as it doesn't work anymore on safari.
probably because it used a positive lookahead regex which is not supported on Safari.
@IonicaBizau can we please adjust the regex in a way that will work on Safari?

the regex:

/(?<!\b(?:[a-z][a-z\d+\-.]{1,50}:))\/{2,}/g

relevant other issue - #22

Thanks!

[IonicaBizau]

Hi @edenhermelin! I am not sure how this should be solved... Ideally, normalize-url should get a patch...

[edenhermelin]

hey @IonicaBizau, thanks for the quick response!
for some reason I thought the regex is defined in parse-url.
I commented on an already-opened issue there: sindresorhus/normalize-url#142 (welcome to upvote if you want 😃 )

Thanks anyway!

[freben]

@IonicaBizau Sorry for commenting on the closed issue, but the README of normalize-url explicitly states that if you want your code to work in a browser environment, you should stay on the 4.x line of normalize-url.

I think that this commit that upgraded normalize-url should be backtracked and made sure to stay on 4.x after that. Unless, of course, you explicitly want some newer features of that library, but if you do, you are also unfortunately also going to have to live with parse-url and git-url-parse being incompatible with widely used browsers.

Maybe a prudent choice could even be to internalize the necessary parts of normalize-url into your library, and dropping that dependency entirely.

[IonicaBizau]

Maybe a prudent choice could even be to internalize the necessary parts of normalize-url into your library, and dropping that dependency entirely.

@freben I thought of that too, but would like to keep it modular...

but the README of normalize-url explicitly states that if you want your code to work in a browser environment, you should stay on the 4.x line of normalize-url.

Can you please send me a link a of that? I couldn't find it...

[freben]

Can you please send me a link a of that? I couldn't find it...

Sure! It's near the top of the readme. Seems to be an intentional choice he made.

He does not seem to respond to the issue about this problem either.

[freben]

Also made an attempt at contributing a fix: sindresorhus/normalize-url#148

[freben]

@IonicaBizau Update: My contributed fix was merged! :) Version 7.0.1 of normalize-url is now released. However, the 7 series is using ESM; read here https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c

Which means that as I bump the dep to ^7.0.1, npm test results in

internal/modules/cjs/loader.js:1080
      throw new ERR_REQUIRE_ESM(filename, parentPath, packageJsonPath);
      ^

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /Users/freben/dev/github/parse-url/node_modules/normalize-url/index.js
require() of ES modules is not supported.
require() of /Users/freben/dev/github/parse-url/node_modules/normalize-url/index.js from /Users/freben/dev/github/parse-url/lib/index.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename /Users/freben/dev/github/parse-url/node_modules/normalize-url/index.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /Users/freben/dev/github/parse-url/node_modules/normalize-url/package.json.

I'll take a break from digging through this right now, maybe you have the ability to continue getting this bubbling up all the way to git-url-parse.

[edenhermelin]

hey @IonicaBizau ,
just wanted to bump this issue in case you are working on a fix.
as @freben mentioned here, the normalize-url library released version 7 that includes a fix - but it looks like it cannot be used in the project..

[IonicaBizau]

@edenhermelin Sorry for the late reply. I'm getting the following error when upgrading to the latest version:

.../parse-url/lib/index.js:4
    , normalizeUrl = require("normalize-url")
                     ^

Error [ERR_REQUIRE_ESM]: require() of ES Module .../parse-url/node_modules/normalize-url/index.js from .../parse-url/lib/index.js not supported.
Instead change the require of .../parse-url/node_modules/normalize-url/index.js in .../parse-url/lib/index.js to a dynamic import() which is available in all CommonJS modules.
  ...
  code: 'ERR_REQUIRE_ESM'
}

I am wondering if there is any simple way to fix this. Is import supported natively in Node.js?

[edenhermelin]

hey, @IonicaBizau thanks for getting back to me!

I know that there are ways to convert the project to use ESM/ to dynamically import ESM dependencies like mentioned in this gist by
sindresorhus (normalize-url's maintainer).

I don't know if these are approaches that you want to do in your project though.

I did clone the repo and tried to just take the index.js from normalize-url and call it from your code, and it seems to work just fine 🤔
I'm wondering if there shouldn't be a way to just minify/transpile normalize-url after it's being installed to just be a commonJS file. I'll try to search more online, but perhaps you can ask
also the community of https://github.com/sindresorhus/normalize-url ?

[edenhermelin]

Hey @IonicaBizau , I pushed a PR that should fix the issue.
Please let me know what do you think :)
locally it seems as it works with the provided example