Skip to content

Fix default http client#201

Merged
kanadgupta merged 4 commits intoreadmeio:mainfrom
AdrianMachado:fixHttpDefault
Sep 21, 2023
Merged

Fix default http client#201
kanadgupta merged 4 commits intoreadmeio:mainfrom
AdrianMachado:fixHttpDefault

Conversation

@AdrianMachado
Copy link
Copy Markdown

@AdrianMachado AdrianMachado commented Sep 20, 2023

🚥 Resolves N/A

🧰 Changes

The default for http does not match any client IDs. This will cause issues when trying to generate a code sample using .convert and trying to use the default as the second param.

🧬 QA & Testing

You can run the following typescript code

import { availableTargets } from "@readme/httpsnippet";

const languageTargets = availableTargets();
const httpLanguageTarget = languageTargets.find(
  (target) => target.key === "http",
); // Can add a ! at the end for type checker
snippet = new HTTPSnippet({
  bodySize = -1,
  cookies: [],
  headers: [],
  headersSize: -1,
  httpVersion: "HTTP/1.1",
  method: "GET",
  queryString: [],
  url: "https://www.example.com/foo",
}).convert(httpLanguageTarget, httpLanguageTarget.default)

Current behavior

TypeError: Cannot destructure property 'convert' of 'target.clientsById[(clientId || target.info.default)]' as it is undefined.
    at HTTPSnippet.convert (webpack-internal:///(app-pages-browser)/./node_modules/@readme/httpsnippet/dist/index.mjs:230:15)

Expected Behavior

Should just work

Copy link
Copy Markdown

@kanadgupta kanadgupta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed up a snapshot and added a comprehensive test to ensure this kind of thing doesn't happen again. thanks for the fix!

@kanadgupta kanadgupta merged commit 2c397e5 into readmeio:main Sep 21, 2023
@AdrianMachado AdrianMachado deleted the fixHttpDefault branch September 21, 2023 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants