Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add normalize-url utility #202

Merged
merged 3 commits into from
Jul 14, 2022

Conversation

thornbill
Copy link
Member

@thornbill thornbill commented Jul 6, 2022

This migrates from including normalize-url as a dependency to include a rewritten version directly as a utility function in the source. The modified version was ported to TypeScript and stripped of any unused functionality.

This was done because the package has been a bit difficult to work with for awhile now...

v7 of normalize-url is only packaged as an ESM module making it difficult to work with in a lot of cases, due to ecosystem support not being great. The developer's position on ESM means this is unlikely to be fixed by an upstream change. This was in fact the entire reason for the v0.3.1 bugfix release.

Unknowingly at the time, reverting to v6 introduced breakages in older browsers and react native due to the use of unsupported regex features. The developer has stated they will not accept backport fixes for v6.

@thornbill thornbill added bug Something isn't working dependencies Pull requests that update a dependency file labels Jul 6, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #202 (9dd310a) into master (4e5caf5) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #202   +/-   ##
=======================================
  Coverage   92.07%   92.07%           
=======================================
  Files          77       77           
  Lines         454      454           
  Branches       43       43           
=======================================
  Hits          418      418           
  Misses         36       36           
Impacted Files Coverage Δ
src/utils/url.ts 100.00% <100.00%> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@thornbill thornbill changed the title Include normalize-url directly in source Add normalize-url utility Jul 13, 2022
@thornbill thornbill merged commit 9cb7ef0 into jellyfin:master Jul 14, 2022
@thornbill thornbill deleted the fix-normalize-version-madness branch July 14, 2022 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants