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

replace node:url with native url? #2291

Closed
jimmywarting opened this issue Oct 14, 2021 · 5 comments · Fixed by #3137
Closed

replace node:url with native url? #2291

jimmywarting opened this issue Oct 14, 2021 · 5 comments · Fixed by #3137

Comments

@jimmywarting
Copy link
Contributor

swagger-js/src/index.js

Lines 86 to 102 in ba0db61

Swagger.prototype.applyDefaults = function applyDefaults() {
const { spec } = this;
const specUrl = this.url;
// TODO: OAS3: support servers here
if (specUrl && specUrl.startsWith('http')) {
const parsed = Url.parse(specUrl);
if (!spec.host) {
spec.host = parsed.host;
}
if (!spec.schemes) {
spec.schemes = [parsed.protocol.replace(':', '')];
}
if (!spec.basePath) {
spec.basePath = '/';
}
}
};

@char0n
Copy link
Member

char0n commented Oct 14, 2021

Yeah, don't see a reason why not.

@char0n
Copy link
Member

char0n commented Dec 14, 2021

I was triaging this issue a bit and came into conclusion that replacing the url Node.js module with WHATWG API will not be that easy.

url.resolve can be replaced with following function, that's no brainer.

// WHATWG URL API replacement for url.resolve function
export function urlResolve(from, to) {
  const resolvedUrl = new URL(to, new URL(from, 'resolve://'));
  if (resolvedUrl.protocol === 'resolve:') {
    // `from` is a relative URL.
    const { pathname, search, hash } = resolvedUrl;
    return pathname + search + hash;
  }
  return resolvedUrl.toString();
}

But url.parse(str) cannot be easily replaced with new URL(str) as the URL constructor expects valid URL as the first parameter and throws error when provided argument is not valid URL.

@char0n
Copy link
Member

char0n commented Dec 14, 2021

Found an inconsistency in WHATWG URL parsing. Before this inconsistency is addressed, let's put this issue on hold.

Inconsistency described here: whatwg/url#677

@char0n
Copy link
Member

char0n commented Sep 12, 2023

url package have been removed in #3137

Codebase now contains helper function for parsing URI References that looks like this:

/**
 * `parseURIReference` function simulates the behavior of `node:url` parse function.
 * New WHATWG URL API is not capable of parsing relative references natively,
 * but can be adapter by utilizing the `base` parameter.
 */
const parseURIReference = (uriReference) => {
  try {
    return new URL(uriReference);
  } catch {
    const parsedURL = new URL(uriReference, 'https://swagger.io');
    const pathname = String(uriReference).startsWith('/')
      ? parsedURL.pathname
      : parsedURL.pathname.substring(1);

    return {
      hash: parsedURL.hash,
      host: '',
      hostname: '',
      href: '',
      origin: '',
      password: '',
      pathname,
      port: '',
      protocol: '',
      search: parsedURL.search,
      searchParams: parsedURL.searchParams,
    };
  }
};

@char0n char0n closed this as completed Sep 12, 2023
char0n added a commit that referenced this issue Sep 12, 2023
char0n added a commit that referenced this issue Sep 12, 2023
swagger-client requires Node.js >=12.20.0 and uses different fetch 
implementation depending on Node.js version:

>=12.20.0 <16.8 - node-fetch@3
>=16.8 <18 - undici
>=18 - native Node.js fetch

Closes #1220
Closes #2736
Closes #2415
Closes #2381
Closes #2187
Closes #2291
swagger-bot pushed a commit that referenced this issue Sep 12, 2023
# [3.21.0](v3.20.2...v3.21.0) (2023-09-12)

### Features

* replace node-fetch with undici ([#3137](#3137)) ([bc7ca17](bc7ca17)), closes [#1220](#1220) [#2736](#2736) [#2415](#2415) [#2381](#2381) [#2187](#2187) [#2291](#2291)
@swagger-bot
Copy link
Contributor

🎉 This issue has been resolved in version 3.21.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants