Total Overhaul (but with all the same fixtures!) #248

dimitropoulos · 2022-04-08T21:40:10Z

Background

Hi!

I'm Dimitri, and I am one of the developers here at Kong working (primarily) on Insomnia. I realize it may not have been clear in the past, but at Kong we consider my team to be the "owners" of httpsnippet.

One of our core values here at Kong is "Real", so I'm gonna just keep it real and say that I realize that this package has been largely abandoned (for the most part) over the last few years. Especially the last year, where there has been plenty of pull requests and issues and almost none were even answered by any maintainers.

The thing is... we feel this pain on Insomnia too. For example, we were looking at some open source issues on our weekly Insomnia livecoding stream and you can just about see the moment when the fun drains out of the room when we realize that to modify a behavior in Insomnia, we're going to have to touch httpsnippet. Yikes.

Thankfully, this wasn't the first time this library had come up recently and a few different people at Kong have been running into problems with this library. And to be fair, it's not only "problems" that we hit, we also have aspirations for how we could leverage this tool better in the future. So, beginning about a week ago, I started taking some time to look at this repo and understand how it worked after work hours. I quickly concluded that there are two very VERY positive things we have going for us:

there are TONS and TONS (I mean like SIX HUNDRED) of fixture tests for all the various clients
the problem-domain of this package is very finite. it's a "solveable" problem (in terms of the functionality the package provides).

So, then, I set about the job of retooling it from the ground up. It's a completely modernized stack. Many of the same nuts and bolts, but now using things like template literals (instead of those ancient and error-prone utils.format magic strings), destructuring, optional chaining, etc. and all tied together with TypeScript in full strict mode! That's not to mention it runs tests in Jest, uses Yargs, Yarn, lots more (and modern) linting, prettier, and we were able to throw out a good few dependencies.

What to do with this PR

This is going to be a major release, for sure, but it's also going to be a pretty hefty breath of life into this project.

Immediately after this PR there are a few things I'd like to do, yet, but I'm making this PR now because I think what's here can merge. I'd like to do the CI with GitHub Actions, for example, and fix the dockerfiles, use jest to report line coverage, etc.

I'd like to do a few betas, maybe, after this releases so we can dogfood this in Insomnia and make sure it's ready for the world.

Here are some examples:

What changed

Check out TODO.txt on the root in this branch to see a list I've been keeping as I go of what had to change. But the quick answer is: not much of any real significance.

How do I review this

The key key key point I want you to know before you do any reviewing is that I didn't touch any of the main input-output fixture tests. This is really important because it means that, at a minimum, the primary behavior (to the character!) of the package has not changed.

I don't want to make light of how much was moved around. If you were familiar with this package before, you'll immediately see that things are organized pretty differently. For example, instead of having 3 places in the repo where the targets/clients structure was duplicated (source code, unit tests, and fixture tests) it's all co-located in the same place.

I kept the commits there for (mainly) my own reference ONLY.

PLEASE DO NOT feel the need to look commit by commit. Everything changed (except the raw fixtures!) so it's most definitely best to just look at it fresh.

What's the plan going forward

I'm super open to feedback!

From the Insomnia team I'm gonna invite @filfreire @DMarby @johnwchadwick @davidma415 @gatzjames @jackkav and @wdawson to take a look and let me know if they have any thoughts. No pressure.

Beyond that, there are a few others that I would love to hear from including @thibaultcha, @darrenjennings, @reynolek, @jeffyongtaotang, @gschier, @nateslo, @develohpanda but I don't have the expectation that any of you will respond, no worries!

Then, I'd like to recognize @erunion. You clearly care about this project and I'd love to talk with you about what we can do to bring you a little more into the fold (i.e. the ability to review and merge PRs. Could you please reach out to me over on the insomnia slack: @Dimitri Mitropoulos.

I'm sure there's lots more to say, but I have a pizza in the oven and I'm hungry, haha. It's been a long (and rewarding!) day so I'm gonna stop there for now. Please feel free to ask any questions (anyone!). Hopefully a bright future to come!

closes: #186

… strings now

I thought I could get away with this but the prior commit with raw http responses taught me that the code-point by code-point output matters quite a lot, including trailing newlines. oh well.

it's not a big deal, but it's cool to make them all the same

filfreire · 2022-04-21T13:46:08Z

Awesome work @dimitropoulos

package.json

dimitropoulos · 2022-04-26T12:31:38Z

Here were my notes:

TODO:

convert to GitHub actions
fix Dockerfile and docker-compose
look into the prior usage of the 'debug' package
write some CLI tests to ensure the behavior is predictable
completely redo the README

later:

use prettier for js languages (and more?) but as a devDependency only to not increase bundle size
add option to curl for minifying json

changelog:

Breaking

drops support for node versions less than 14.19 (the current LTS)
the http target changed its client ID to http1.1 from 1.1
formatter strings (i.e. "%s" substitutions) are no longer supported (now that template strings exist, you don't need them, so please just send the string you want as the line of code as-is)
the "dot-method-train" style API is no longer supported on the CodeBuilder. All the same functionality exists, but just call the methods separately. For example instead of code.push('a').blank().push('b') just do push('a'); blank(); push('b');
the JavaScript target will output ES6 code
HTTPSnippet is no longer the default export (now a named export)

* Total Overhaul (but with all the same fixtures!) (#248) see Kong#248 and the commits therein for more context. Essentially: - the original client fixtures were unchanged, which hopefully means the - all source code is now in strict mode TypeScript - tests are now all in jest - the file structure was reorganized so that everything for a particular client is in one place - the CLI is updated and now using yargs - all dependencies were updated and some (i.e. `format.utils`) were able to be removed entirely - more work left to do (including CI with GitHub Actions, for example), but this is a start * fix: case where if `postData.params` is missing some targets crash (#258) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * fix: compatibility issues on node 14 with `Object.hasOwn()` (#252) * fix: typo in the httpie `style` option not being correctly applied (#254) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * fix: axios targets not sending `x-www-form-urlencoded` properly (#255) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * feat: addition of a PHP target for Guzzle (#253) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * Add Github Build Workflow (#250) (#251) * Add Github Build Workflow (#250) * Edit job name * Replace install with ci on GH workflow * Add matrix with major node versions (14, 16, 18) * Disable fail-fast * Remove node v14 from build GH action * feat: native upload support in python `requests` snippets (#259) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * fix: `multipart/form-data` header issues with node/js fetch targets (#257) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * fix: headers not being properly applied to R httr snippets (#263) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * Fix build workflow dispatch rules (#265) * Chore: Remove travis links (#266) * Remove travis links * Update README.md Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * chore: fixing eslint issues * fix: updating test snapshots * fix: getting the stock target tests all passing * fix: broken snapshots * fix: adding missing test coverage for addTarget and addTargetClient * fix: build issues * feat: getting the integration tests suite running again * fix: issues with the docker setup * fix: removing unnecessary tests * fix: running docker tests * fix: typo * fix: reverting some query param changes to node axios + request * fix: docker issues * fix: cleaning up the integration suite config system Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> Co-authored-by: Filipe Freire <livrofubia@gmail.com>

* Total Overhaul (but with all the same fixtures!) (#248) see Kong#248 and the commits therein for more context. Essentially: - the original client fixtures were unchanged, which hopefully means the - all source code is now in strict mode TypeScript - tests are now all in jest - the file structure was reorganized so that everything for a particular client is in one place - the CLI is updated and now using yargs - all dependencies were updated and some (i.e. `format.utils`) were able to be removed entirely - more work left to do (including CI with GitHub Actions, for example), but this is a start * fix: case where if `postData.params` is missing some targets crash (#258) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * fix: compatibility issues on node 14 with `Object.hasOwn()` (#252) * fix: typo in the httpie `style` option not being correctly applied (#254) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * fix: axios targets not sending `x-www-form-urlencoded` properly (#255) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * feat: addition of a PHP target for Guzzle (#253) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * Add Github Build Workflow (#250) (#251) * Add Github Build Workflow (#250) * Edit job name * Replace install with ci on GH workflow * Add matrix with major node versions (14, 16, 18) * Disable fail-fast * Remove node v14 from build GH action * feat: native upload support in python `requests` snippets (#259) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * fix: `multipart/form-data` header issues with node/js fetch targets (#257) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * fix: headers not being properly applied to R httr snippets (#263) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * Fix build workflow dispatch rules (#265) * Chore: Remove travis links (#266) * Remove travis links * Update README.md Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * fix: issue where query strings in R wouldn't be properly concatenated (#269) * fix: issue where query strings in R wouldn't be properly concatenated * adds (and respects) indent options to httr, plus double looping fix now, indent is respected, and also avoiding running Object.keys twice per run since we can just run it once with .entries Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * add header namesspace to prevent header errors (#247) * add header namesspace to prevent header errors * update fixtures Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * fix: stop implicitly coercing warning in Swift snippet generator (#195) * swift/nsurlsession adds `as Any` to print for error * adds OVERWRITE_EVERYTHING to ease fixture snapshot resetting * updates fixtures Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * fix: clj-http handling of literal null JSON bodies (#283) Co-authored-by: Sergey Zakharchenko <szakharchenko@digital-loggers.com> * fix: prevent crash in Swift/Objc with checking length of input body post params (#192) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * fix: cUrl target should encode x-www-form-urlencoded post data params (#198) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * fix: linting issues * fix: reverting package-lock changes * chore: removing unused fixtures * fix: removing unnecessary typing Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> Co-authored-by: Filipe Freire <livrofubia@gmail.com> Co-authored-by: Davis Martin <davis.martin@procore.com> Co-authored-by: iraj taghlidi <785830+irajtaghlidi@users.noreply.github.com> Co-authored-by: Sergey Zakharchenko <szakharchenko@digital-loggers.com> Co-authored-by: Julien Giovaresco <dev@giovaresco.fr>

* Total Overhaul (but with all the same fixtures!) (#248) see Kong#248 and the commits therein for more context. Essentially: - the original client fixtures were unchanged, which hopefully means the - all source code is now in strict mode TypeScript - tests are now all in jest - the file structure was reorganized so that everything for a particular client is in one place - the CLI is updated and now using yargs - all dependencies were updated and some (i.e. `format.utils`) were able to be removed entirely - more work left to do (including CI with GitHub Actions, for example), but this is a start * fix: case where if `postData.params` is missing some targets crash (#258) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * fix: compatibility issues on node 14 with `Object.hasOwn()` (#252) * fix: typo in the httpie `style` option not being correctly applied (#254) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * fix: axios targets not sending `x-www-form-urlencoded` properly (#255) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * feat: addition of a PHP target for Guzzle (#253) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * Add Github Build Workflow (#250) (#251) * Add Github Build Workflow (#250) * Edit job name * Replace install with ci on GH workflow * Add matrix with major node versions (14, 16, 18) * Disable fail-fast * Remove node v14 from build GH action * feat: native upload support in python `requests` snippets (#259) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * fix: `multipart/form-data` header issues with node/js fetch targets (#257) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * fix: headers not being properly applied to R httr snippets (#263) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * Fix build workflow dispatch rules (#265) * Chore: Remove travis links (#266) * Remove travis links * Update README.md Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * fix: issue where query strings in R wouldn't be properly concatenated (#269) * fix: issue where query strings in R wouldn't be properly concatenated * adds (and respects) indent options to httr, plus double looping fix now, indent is respected, and also avoiding running Object.keys twice per run since we can just run it once with .entries Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * add header namesspace to prevent header errors (#247) * add header namesspace to prevent header errors * update fixtures Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * fix: stop implicitly coercing warning in Swift snippet generator (#195) * swift/nsurlsession adds `as Any` to print for error * adds OVERWRITE_EVERYTHING to ease fixture snapshot resetting * updates fixtures Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * fix: clj-http handling of literal null JSON bodies (#283) Co-authored-by: Sergey Zakharchenko <szakharchenko@digital-loggers.com> * fix: prevent crash in Swift/Objc with checking length of input body post params (#192) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * fix: cUrl target should encode x-www-form-urlencoded post data params (#198) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * feat: Add support for insecureSkipVerify (#285) * go/native: adds insecureSkipVerify * node/native: adds insecureSkipVerify * python/python3: adds insecureSkipVerify * ruby/native: adds insecureSkipVerify * shell/curl: adds insecureSkipVerify Co-authored-by: Tim Perry <pimterry@gmail.com> * feat: implementing cleaner handling of JSON in cURL snippets (#256) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * chore: minor cleanup (#286) * feat: use curl's --compressed option for requests that accept it (#287) * feat: make Python snippets simpler, clearer & more consistent (#288) Co-authored-by: Tim Perry <pimterry@gmail.com> * feat: change the default response code for Python Requests (#181) Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * feat: PHP JSON body encoding (#291) * php/curl: use json_encode for CURLOPT_POSTFIELDS * php/http1: use json_encode when body is JSON * php/http2: use json_encode when body is JSON Co-authored-by: Andrii Kostenko <andrey@kostenko.name> * Async/Await (top level) support in JavaScript snippets (#292) * Exclude package.json from build to fix output paths (#294) * Exclude package.json from build to fix output paths * keeps bin pointing at cli output * makes rootDir explicit * removes unused cli build scripts Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> * Fix crash when building nsurlsession snippets for empty params (#295) * removes `require 'openssl'` from ruby target (no longer needed) (#296) * Escape quotes in headers correctly in all languages (#289) * updates README (#299) Co-authored-by: Filipe Freire <livrofubia@gmail.com> * ioutil -> io (deprecated) (#305) * chore: undoing unwanted changes * chore: revert more unwanted changes * chore: reverting more unwanted changes * fix: fixing broken test snapshots and libcurl not escaping * fix: a bunch of broken tests * fix: removing dead code * fix: remove support for `insecureSkipVerify` as we dont need or want it * fix: removing top-level await changes for axios * fix: revert top-level await changes for js:fetch * fix: remove some problematic changes to `node:request` * fix: retaining line trimming in powershell snippets * fix: bug in php snippets where booleans were casted to null * fix: revert problematic changes to python:requests * fix: revert more unwanted changes * chore: temporarily skipping the integration suite * fix: broken snapshot * fix: disabling the integration suite from being run without inside docker * fix: integration suite * fix: integration suite --------- Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com> Co-authored-by: Filipe Freire <livrofubia@gmail.com> Co-authored-by: Davis Martin <davis.martin@procore.com> Co-authored-by: iraj taghlidi <785830+irajtaghlidi@users.noreply.github.com> Co-authored-by: Sergey Zakharchenko <szakharchenko@digital-loggers.com> Co-authored-by: Julien Giovaresco <dev@giovaresco.fr> Co-authored-by: Tim Perry <pimterry@gmail.com> Co-authored-by: Andrii Kostenko <andrey@kostenko.name> Co-authored-by: Tim Perry <1526883+pimterry@users.noreply.github.com> Co-authored-by: Alexander Weber <aw@voidpointergroup.com>

* Add nvmrc and set it to 14.9 as per #248 * Use node 18 * bump to node 20 on nvmrc file --------- Co-authored-by: Filipe Freire <livrofubia@gmail.com>

dimitropoulos added 30 commits April 8, 2022 17:05

yarn + tsc

9485769

remove all (now unnecessary) 'use-strict'

312bcef

mashape -> kong

318b3e2

prettier

12233d3

fixes malformed json fixtures

edf770b

starts with helpers

3c9e44b

removes string interpolation nonsense. it's 2022 and we have template…

ae355de

… strings now

pretty good for just the first day. stopping for now.

a3aa5ef

oh yeah, and removes useless empty unit tests

10a097c

developing first target and client

e92f6f2

colocates fixtures

8512f4a

c:libcurl

aa3b517

clojure:clj_http

6fc2c87

csharp:httpclient

013758b

csharp:restsharp

d9b2d1e

go:native

a8a2a8a

allow custom fixtures (why wasn't this a thing before?!?!)

bf5e1eb

http:http1.1

4e775f2

removes trailing newlines from fixtures.... lesson learned

ff98045

I thought I could get away with this but the prior commit with raw http responses taught me that the code-point by code-point output matters quite a lot, including trailing newlines. oh well.

java:asynchttp, java:nethpttp, java:okhttp, java:unirest

33990a5

javascript:axios, javascript:fetch, javascript:jquery, javascript:xhr

2891e7a

kotlin:okhttp

e10e0ba

node:axios, node:fetch, node:native, node:request, node:unirest

b59641d

objc:nsurlsession

a7e7a85

ocaml:cohttp

7579d4b

php:curl, php:http1, php:http2

5458d03

powershell:restmethod, powershell:webrequest

9344180

python:python3, python:requests

5580b62

consistent filenames

8b32805

all clients for r, ruby, shell, and swift

61bcf99

dimitropoulos added 6 commits April 13, 2022 15:13

updates node version to maintenance LTS (not just active LTS)

df36e95

use type import syntax per review feedback

d0761ce

fixes type error

6f26447

rearrange runCustomFixture order, per review feedback

6f6a188

it's not a big deal, but it's cool to make them all the same

resolve type merge

7ea5e5d

adds context/documentation

2ffb3da

dimitropoulos mentioned this pull request Apr 13, 2022

Add target for Reqwest library for Rust #242

Closed

filfreire approved these changes Apr 21, 2022

View reviewed changes

dimitropoulos added 2 commits April 21, 2022 13:28

switch to npm 😭

16be920

lint

2925c2e

erunion reviewed Apr 21, 2022

View reviewed changes

package.json Outdated Show resolved Hide resolved

updates permissible node versions

46e2bc7

dimitropoulos force-pushed the overhaul branch from 5be827d to 46e2bc7 Compare April 26, 2022 12:24

removes temporary todo.txt

11c6159

dimitropoulos merged commit 66baca6 into Kong:master Apr 26, 2022

dimitropoulos deleted the overhaul branch April 26, 2022 12:30

This was referenced Apr 26, 2022

Rewrite the readme #249

Closed

make a simple GitHub Actions workflow #250

Closed

bbbco added a commit to bbbco/httpsnippet that referenced this pull request May 13, 2022

Add nvmrc and set it to 14.9 as per Kong#248

489f36d

bbbco mentioned this pull request May 13, 2022

chore: add nvmrc and set it to 20 #262

Merged

This was referenced Jul 13, 2022

Add %v and %q options to format in CodeBuilder.buildLine and some handy indentation methods #246

Closed

Selection of test setup improvements #239

Closed

Updating to consts and arrow functions #116

Closed

dimitropoulos mentioned this pull request Jul 19, 2022

Javascript conversion crashes during multipart form data #88

Closed

filfreire added a commit that referenced this pull request Jul 12, 2024

chore: add nvmrc and set it to 20 (#262)

147eb98

* Add nvmrc and set it to 14.9 as per #248 * Use node 18 * bump to node 20 on nvmrc file --------- Co-authored-by: Filipe Freire <livrofubia@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Total Overhaul (but with all the same fixtures!) #248

Total Overhaul (but with all the same fixtures!) #248

Uh oh!

dimitropoulos commented Apr 8, 2022 •

edited

Loading

Uh oh!

filfreire commented Apr 21, 2022

Uh oh!

Uh oh!

dimitropoulos commented Apr 26, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Total Overhaul (but with all the same fixtures!) #248

Total Overhaul (but with all the same fixtures!) #248

Uh oh!

Conversation

dimitropoulos commented Apr 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

What to do with this PR

What changed

How do I review this

What's the plan going forward

Uh oh!

filfreire commented Apr 21, 2022

Uh oh!

Uh oh!

dimitropoulos commented Apr 26, 2022

TODO:

later:

changelog:

Breaking

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dimitropoulos commented Apr 8, 2022 •

edited

Loading