medialize / medialize/URI.js

Make invalid port check optional to support url templates

Open
#417 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
6.2k
Forks
468
PR merge metrics
No merged PRs in 30d

Description

After version 1.18.11 of urijs, an invalid port exception is thrown if the uri is constructed with a port that isn't valid. This check is useful to avoid having an invalid value as a port, increasing the correctness of the generated URI, but makes impossible to use the library when working with url templates.

Before 1.18.11, the following code snippet was possible:

const uri = new urijs('https://github.com:{port}');

After 1.18.11, the same code snippet throws the invalid port exception.

There're several possible solutions to this problem, in case there's the desire to support URI templates.

  1. Make port check optional by passing an options object to the constructor:
const uri = new urijs('https://github.com:{port}', { ensureValidPort: false });
  1. The second idea that comes to my mind is to throw the exception not when the URI is created but when it's converted to a string. My hypothesis here is that uri.toString() or uri.valueOf() might be the last calls to the urijs library before the constructed URI is used.
const uri = new urijs('https://github.com:{port}'); // valid, no exception is thrown
// do stuff here
const resp = await fetch(uri.toString()); // here an exception will be thrown if the port is still invalid
  1. There might be also other valid options.

Would such change be considered for this library?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reproducing the invalid-port exception with new urijs('https://github.com:{port}') on version 1.18.11 or later. Review the constructor and URI string-conversion behavior to compare the proposed options, then define and test one consistent approach for supporting URL templates while preserving invalid-port validation.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
web-dev
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.