Replace got with axios and allow isomorphic export #172

KenEucker · 2021-04-17T21:09:33Z

This PR includes a heavy rewrite of the core of imgur@next, moving away from got and to axios which can be compiled and used in the browser. This, in conjunction with dropping support for "local file" uploads by using fs.createReadStream, allows this package to be used in nodejs and in the browser after being compiled as a umd module.

Tasks completed:

Replace got with axios. Replacing Response wrapping.
Add a helper method to utils.ts convert from the underlying response object to an ImgurApiResponse.
Replace the use of path strings resulting in a call to createReadStream, with the string parameter to be interpreted as a "url" upload.
Remove upload methods specific to videos and files in favor of streams for these upload types.
Update the readme to show the external use of the fs.createReadStream for local file uploads using the API.
Add readme instructions for creating streams in the browser and uploading via the browser.
Reconfigure the build process for the package to include webpack bundling as a umd module.
Update tests.

The only item missing from this PR that isn't complete feature parity with the current imgur@next is the lack of progress updates for uploaded items. The test for that would not succeed and I wasn't able to determine why but hope to find time to address it in the future.

@kaimallea I know that I was the one to suggest using got in the first place, and I know that you had said on a previous issue that supporting node was the singular focus of this package. The reason that I have completed this work is that I have a need for this package to be able to be used in the client, browser, as well and I found, in my discovery of trying to do so, that the only negative impact is removing a single call using fs. Please let me know if this is something you are willing to consider?

Merging upstream changes.

Merging main from origin

Merged upstream

Merged version 1 from upstream

Bumps [msw](https://github.com/mswjs/msw) from 0.27.1 to 0.27.2. - [Release notes](https://github.com/mswjs/msw/releases) - [Changelog](https://github.com/mswjs/msw/blob/master/CHANGELOG.md) - [Commits](mswjs/msw@v0.27.1...v0.27.2) Signed-off-by: dependabot[bot] <[email protected]>

…27.2 build(deps-dev): bump msw from 0.27.1 to 0.27.2

Bumps [msw](https://github.com/mswjs/msw) from 0.27.2 to 0.28.0. - [Release notes](https://github.com/mswjs/msw/releases) - [Changelog](https://github.com/mswjs/msw/blob/master/CHANGELOG.md) - [Commits](mswjs/msw@v0.27.2...v0.28.0) Signed-off-by: dependabot[bot] <[email protected]>

Bumps [eslint](https://github.com/eslint/eslint) from 7.22.0 to 7.23.0. - [Release notes](https://github.com/eslint/eslint/releases) - [Changelog](https://github.com/eslint/eslint/blob/master/CHANGELOG.md) - [Commits](eslint/eslint@v7.22.0...v7.23.0) Signed-off-by: dependabot[bot] <[email protected]>

…28.0 build(deps-dev): bump msw from 0.27.2 to 0.28.0

Bumps [husky](https://github.com/typicode/husky) from 5.2.0 to 6.0.0. - [Release notes](https://github.com/typicode/husky/releases) - [Commits](typicode/husky@v5.2.0...v6.0.0) Signed-off-by: dependabot[bot] <[email protected]>

…-7.23.0 build(deps-dev): bump eslint from 7.22.0 to 7.23.0

…6.0.0 build(deps-dev): bump husky from 5.2.0 to 6.0.0

Bumps [@commitlint/config-conventional](https://github.com/conventional-changelog/commitlint) from 12.0.1 to 12.1.1. - [Release notes](https://github.com/conventional-changelog/commitlint/releases) - [Changelog](https://github.com/conventional-changelog/commitlint/blob/master/CHANGELOG.md) - [Commits](conventional-changelog/commitlint@v12.0.1...v12.1.1) Signed-off-by: dependabot[bot] <[email protected]>

Bumps [msw](https://github.com/mswjs/msw) from 0.28.0 to 0.28.1. - [Release notes](https://github.com/mswjs/msw/releases) - [Changelog](https://github.com/mswjs/msw/blob/master/CHANGELOG.md) - [Commits](mswjs/msw@v0.28.0...v0.28.1) Signed-off-by: dependabot[bot] <[email protected]>

…lint/config-conventional-12.1.1 build(deps-dev): bump @commitlint/config-conventional from 12.0.1 to 12.1.1

Bumps [@commitlint/cli](https://github.com/conventional-changelog/commitlint) from 12.0.1 to 12.1.1. - [Release notes](https://github.com/conventional-changelog/commitlint/releases) - [Changelog](https://github.com/conventional-changelog/commitlint/blob/master/CHANGELOG.md) - [Commits](conventional-changelog/commitlint@v12.0.1...v12.1.1) Signed-off-by: dependabot[bot] <[email protected]>

…28.1 build(deps-dev): bump msw from 0.28.0 to 0.28.1

…lint/cli-12.1.1 build(deps-dev): bump @commitlint/cli from 12.0.1 to 12.1.1

Merge next-dev into next

Bumps [eslint](https://github.com/eslint/eslint) from 7.23.0 to 7.24.0. - [Release notes](https://github.com/eslint/eslint/releases) - [Changelog](https://github.com/eslint/eslint/blob/master/CHANGELOG.md) - [Commits](eslint/eslint@v7.23.0...v7.24.0) Signed-off-by: dependabot[bot] <[email protected]>

…-7.24.0 build(deps-dev): bump eslint from 7.23.0 to 7.24.0

Bumps [eslint-config-prettier](https://github.com/prettier/eslint-config-prettier) from 8.1.0 to 8.2.0. - [Release notes](https://github.com/prettier/eslint-config-prettier/releases) - [Changelog](https://github.com/prettier/eslint-config-prettier/blob/main/CHANGELOG.md) - [Commits](prettier/eslint-config-prettier@v8.1.0...v8.2.0) Signed-off-by: dependabot[bot] <[email protected]>

…-config-prettier-8.2.0 build(deps-dev): bump eslint-config-prettier from 8.1.0 to 8.2.0

…h both the browser and nodejs This update abstracts the http request method out a little futher than it was in a "fetcher" object and includes a utility wrapper for conforming request responses to the ImgurApiResponse model. All test have been updated and support for automatic filepath uploads has been dropped in favor of cross-platform support. BREAKING CHANGE: The package can no longer be used with string path variables pointing to local files. This moves a single line of code, using fs.createReadStream, out of the project and places that responsibility on the consumer. The package now supports streams in favor of the string filepath upload.

KenEucker · 2021-04-17T21:40:55Z

The tests are failing, due to a warning I guess?, because of this line in authorize.ts:

The offending code:
const { username, password, allow } = req.body as any

req.body, with axios, is an object in this case. I might add an eslint ignore to this line?

… mock for authorization this should be revisited in the future to determine the correct type of the body in order to deconstruct it.

…t script in package.json This update includes modifications to the scripts in package.json to allow users to run the pretty and eslint tasks individually, and has the lint script call those individual scripts together. All scripts now use npx to instruct npm to use the module from node_modules.

KenEucker · 2021-04-20T01:47:37Z

Something is weird with the PR, and I think it's getting hung up on something related to the action's permissions. Maybe because this is coming from a branch on my repository it doesn't have access to the same secrets you have set up in yours, @kaimallea?

If I'm reading these errors correctly, it looks like the tests are failing when attempting to send a message back to GitHub stating that tests succeeded:

…e ImgurClient class To provide additional context and type support for projects consuming the node-imgur api, who might be storing ImgurCredentials in an object of their

kaimallea · 2021-04-20T15:17:31Z

Universal/isomorphic support will be amazing. There's a lot to digest here, I will dig in after work hours! Thanks for the PR 🙌🏾

KenEucker · 2021-04-21T17:38:27Z

@kaimallea I don't know if you have already taken a look at this code or not, but I wanted to provide additional context after more testing:

It seems that the UMD strategy I was using to create the bundle works fine for the browser (it is a webpack bundle, after all), but it doesn't work with node. I spent some time last night digging through the issue and will need to update this PR once again when I find the right configuration combination to produce the results we're after.

I am expecting to be able to have a single index.ts file be the entry point for both the node and the web versions, and for webpack to produce a single dist file to be consumed by both node and the browser, with the following instantiation code:

web

<script src="path-to-local-library/index.js"></script>
<script>
const imgurClient = imgur({})
</script>

or with bundlers
import { ImgurClient } from 'imgur';
const imgur = require('imgur');

node

// ESModule syntax
import { ImgurClient } from 'imgur'

// CommonJS syntax
const { ImgurClient } = require('imgur')

This is work I am committed to completing for the BikeTag API and would love to be able to bring it back to this project.

Universal Javascript is bae, Isomorphic Support is life.

kaimallea

I think I would have preferred a more iterative approach to limit the total number of changes happening at once. For example, just replacing got with axios, and no other changes, to isolate those changes and tests. And then after that's done and merged, more iterative changes to make it more isomorphic/universal. I think that makes it easier/faster to make changes, and review, too.

kaimallea · 2021-04-24T22:48:43Z

package.json

+    "test": "npx jest",
+    "test:ci": "npx jest --ci --coverage --runInBand",
+    "build": "npx tsc",
+    "compile": "npx webpack",
+    "typecheck": "npx tsc --noEmit",
+    "eslint": "npx eslint . ",
+    "pretty": "npx prettier . --check ",
+    "pretty:write": "npx prettier .  --write ",
    "clean": "rm -rf lib",
-    "prepare": "husky install",
-    "lint": "eslint . && prettier --check . && npm run typecheck",
-    "commit": "cz"
+    "prepare": "npx husky install",
+    "lint": "npm run eslint && npm run pretty && npm run typecheck",
+    "commit": "npx cz"


why are you adding npx in front of all of these commands? these are dev dependencies, so after the initial npm install they will be automatically resolved via node_modules/,bin. Reference.

I thought that when running npm scripts that it used the most local node_modules, but when I tried running it with this and other projects recently it does not. I found that I either had to install eslint globally or these commands would not work for me. Maybe that's a my machine problem?

kaimallea · 2021-04-24T22:55:44Z

src/common/utils.ts

  if (typeof payload === 'string') {
    return false;
  }

-  return typeof payload.video === 'string';


how is someone supposed to upload a video with this removed? The Imgur API requires that the video property is set in the formdata. sure, you can stream in a video file, but how is Imgur API supposed to differentiate a video from an image?

I assume through a stream, but this line looks incorrect. I think this should be compared to a type of stream.

The code will pass along the video as a stream into the form to be set as the 'video' field, from what I understand of it.

kaimallea · 2021-04-24T22:59:29Z

src/common/utils.ts

  if (typeof payload === 'string') {
    return payload;
  }

-  if (isVideo(payload)) {
-    return payload.video as string;
+  if (isBase64(payload)) {


base64 is already supported, so not sure why it's even more explicit? is this to better support browser environment?

I added this method following what I saw from the code for checking other types. I thought this would add more of the same typechecking. If this is not needed, then it should be removed.

kaimallea · 2021-04-24T23:05:31Z

src/mocks/handlers/index.ts

@@ -8,9 +8,17 @@ import * as gallery from './gallery';
 import * as credits from './credits';
 import * as album from './album';

+// import axios from 'axios';
+// import MockAdapter from 'axios-mock-adapter';
+// const mock = new MockAdapter(axios);


Sorry, I didn't realize that these lines were in this file. This should also be removed.

kaimallea · 2021-04-24T23:07:05Z

src/common/utils.ts

    } else {
      form.append(key, value);
    }
  }
  return form;
 }
+
+export function getImgurApiResponseFromResponse(


is the idea that this replace resolveBodyOnly prop from got?

@kaimallea I suppose so. I feel that a helper method here makes the package a bit more modular. This might be solvable by an option with axios that I'm not aware of.

kaimallea · 2021-04-24T23:07:29Z

src/image/upload.test.ts

      title: 'dank meme',
      description: 'the dankiest of dank memes',
    });
    expect(response).toMatchInlineSnapshot(`
      Object {
        "data": Object {
          "deletehash": "jyby9KJ",
-          "description": "the dankiest of dank memes",
+          "description": null,


Woops. It looks like I bungled a jest -u here and didn't resolve it?

kaimallea · 2021-04-24T23:07:31Z

src/image/upload.test.ts

          "id": "JK9ybyj",
          "link": "https://i.imgur.com/JK9ybyj.jpg",
-          "title": "dank meme",
+          "title": null,


KenEucker · 2021-04-24T23:43:27Z

@kaimallea I hear you. I can break up the changes, but what I thought I had to do was make all of the tests pass before I could make a commit. How are you envisioning I could break up this commit into something more iterative?

kaimallea · 2021-04-26T04:01:16Z

Going to make a draft PR as an example of something more iterative

…te targets: node and browser

…piled

KenEucker · 2021-10-29T07:02:05Z

@kaimallea I wanted to circle back with you after recent success in using this version of node-imgur next with axios including isomorphic compilation using webpack. You can see it both here: https://github.com/KenEucker/node-imgur/tree/next and as a release here: https://github.com/KenEucker/node-imgur/tree/release

I am using this version of this package so that the BikeTag API is also isomorphic. You can see it being included in this project: https://github.com/KenEucker/biketag-api.

The tests are failing due to a lack of access, it seems, so those might need to be run by your user account and not mine.

If you get a chance to read this I hope you can consider this PR once again.

…s when sending formData w/axios

this package is now being published under imanagur, as it has diverged from node-imgur, and node-imgur remains stale.

KenEucker and others added 30 commits March 8, 2021 21:34

Merge pull request #6 from kaimallea/main

178eae6

Merging upstream changes.

Merge pull request #9 from kaimallea/main

f8c9cb4

Merging main from origin

Merge pull request #11 from kaimallea/main

dbb8435

Merged upstream

Merge pull request #14 from kaimallea/main

b762236

Merged version 1 from upstream

Merge pull request #147 from kaimallea/dependabot/npm_and_yarn/msw-0.…

2f6943f

…27.2 build(deps-dev): bump msw from 0.27.1 to 0.27.2

Merge pull request #148 from kaimallea/dependabot/npm_and_yarn/msw-0.…

e91d5e0

…28.0 build(deps-dev): bump msw from 0.27.2 to 0.28.0

Merge branch 'main' into dependabot/npm_and_yarn/eslint-7.23.0

299d28f

Merge pull request #151 from kaimallea/dependabot/npm_and_yarn/eslint…

4f7b2fa

…-7.23.0 build(deps-dev): bump eslint from 7.22.0 to 7.23.0

Merge branch 'main' into dependabot/npm_and_yarn/husky-6.0.0

74d2ccf

Merge pull request #152 from kaimallea/dependabot/npm_and_yarn/husky-…

b51b555

…6.0.0 build(deps-dev): bump husky from 5.2.0 to 6.0.0

Merge pull request #153 from kaimallea/dependabot/npm_and_yarn/commit…

5cb4259

…lint/config-conventional-12.1.1 build(deps-dev): bump @commitlint/config-conventional from 12.0.1 to 12.1.1

Merge branch 'main' into dependabot/npm_and_yarn/msw-0.28.1

7734d97

Merge pull request #154 from kaimallea/dependabot/npm_and_yarn/msw-0.…

98c1433

…28.1 build(deps-dev): bump msw from 0.28.0 to 0.28.1

Merge branch 'main' into dependabot/npm_and_yarn/commitlint/cli-12.1.1

978ad5b

Merge pull request #155 from kaimallea/dependabot/npm_and_yarn/commit…

32a1afd

…lint/cli-12.1.1 build(deps-dev): bump @commitlint/cli from 12.0.1 to 12.1.1

Merge pull request #166 from kaimallea/next-dev

3ef9248

Merge next-dev into next

Merge pull request #167 from kaimallea/dependabot/npm_and_yarn/eslint…

1d2f6ef

…-7.24.0 build(deps-dev): bump eslint from 7.23.0 to 7.24.0

Merge pull request #169 from kaimallea/dependabot/npm_and_yarn/eslint…

6ec2fb2

…-config-prettier-8.2.0 build(deps-dev): bump eslint-config-prettier from 8.1.0 to 8.2.0

Merge branch 'main' into main

cc3981e

Merge remote-tracking branch 'upstream/next' into next

e5f323b

KenEucker requested a review from kaimallea April 17, 2021 21:09

KenEucker self-assigned this Apr 17, 2021

KenEucker changed the base branch from main to next-dev April 17, 2021 21:09

KenEucker added 3 commits April 19, 2021 09:35

fix(authorize.ts): adds eslint ignore for an explicit any in the test…

f4ec54b

… mock for authorization this should be revisited in the future to determine the correct type of the body in order to deconstruct it.

Merge branch 'next-dev' into next

bb80487

refactor(client): exports the ImgurCredentials type in addition to th…

4739097

…e ImgurClient class To provide additional context and type support for projects consuming the node-imgur api, who might be storing ImgurCredentials in an object of their

kaimallea changed the base branch from next-dev to next April 23, 2021 23:02

kaimallea changed the base branch from next to next-dev April 23, 2021 23:02

kaimallea reviewed Apr 24, 2021

View reviewed changes

KenEucker added 4 commits October 27, 2021 21:31

build(webpack): splits the compilation of the library into two separa…

d311cc8

…te targets: node and browser

chore(package.json): adds an expose script

a91fe28

build(webpack): changes how the library is exported and how it is com…

0c9ea64

…piled

chore(package.json): cleans up and removes use of npx

a779c10

KenEucker changed the title ~~Replace got with axios and add cross-platform support for this imgur package.~~ Replace got with axios and allow isomorphic export Oct 29, 2021

KenEucker mentioned this pull request Nov 10, 2021

[BUG] RequestError The body option must be a stream.Readable, string or Buffer #233

Closed

KenEucker added 4 commits November 10, 2021 21:41

fix(utils): resolves issue with url uploads

bbaa974

fix(src/image): resolves issues with uploads by adding form.getHeader…

a3c5a53

…s when sending formData w/axios

build(gitignore): ignores build files for the next branch

335d005

refactor(*): imgur -> imanagur

9997530

this package is now being published under imanagur, as it has diverged from node-imgur, and node-imgur remains stale.

KenEucker closed this Dec 8, 2021

KenEucker deleted the next branch December 8, 2021 00:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace got with axios and allow isomorphic export #172

Replace got with axios and allow isomorphic export #172

KenEucker commented Apr 17, 2021

KenEucker commented Apr 17, 2021

KenEucker commented Apr 20, 2021 •

edited

Loading

kaimallea commented Apr 20, 2021

KenEucker commented Apr 21, 2021

kaimallea left a comment •

edited

Loading

kaimallea Apr 24, 2021

KenEucker Apr 24, 2021

kaimallea Apr 24, 2021

KenEucker Apr 24, 2021

kaimallea Apr 24, 2021

KenEucker Apr 24, 2021

kaimallea Apr 24, 2021

KenEucker Apr 24, 2021

kaimallea Apr 24, 2021

KenEucker Apr 24, 2021

kaimallea Apr 24, 2021

KenEucker Apr 24, 2021

kaimallea Apr 24, 2021

KenEucker commented Apr 24, 2021

kaimallea commented Apr 26, 2021

KenEucker commented Oct 29, 2021

Replace got with axios and allow isomorphic export #172

Replace got with axios and allow isomorphic export #172

Conversation

KenEucker commented Apr 17, 2021

KenEucker commented Apr 17, 2021

KenEucker commented Apr 20, 2021 • edited Loading

kaimallea commented Apr 20, 2021

KenEucker commented Apr 21, 2021

kaimallea left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KenEucker commented Apr 24, 2021

kaimallea commented Apr 26, 2021

KenEucker commented Oct 29, 2021

KenEucker commented Apr 20, 2021 •

edited

Loading

kaimallea left a comment •

edited

Loading