infinitered / apisauce

Axios + standardized errors + request/response transforms.
MIT License
2.8k stars 184 forks source link

Add UMD build to dist #260

Closed eithe closed 3 years ago

eithe commented 3 years ago

This change adds a UMD build output which enables legacy projects to use apisauce. This should also resolve #114 as long as axios and ramda is referenced before apisauce.

I have a few legacy projects at work that we are slowly trying to modernize, and transitioning to axios is one step along that path. Coming from react native and you're app templates, I've been very happy with apisauce so hope to also use it for my current scenario :)

This is non-breaking change as the original dist/apisauce.js is unchanged.

I'm open to any feedback, and would greatly appreciate if you would consider including this.

PS: Locally I had to pin @types/node to version 14.0.4 to make apisauce (unchanged) compile, but didn't include that change here since might just be me. Might be related to https://github.com/DefinitelyTyped/DefinitelyTyped/issues/45116

PS2: Since apisauce uses a somewhat old version of rollup I had to resort to corresponding (and also deprecated) versions of the commonjs and node-resolve rollup plugins.

eithe commented 3 years ago

Ok, so the same error related to @types/node mentioned in my first "PS" happened on the circli test. I don't think this is due to what I have added since it also occured when I tried to compile apisauce unchanged locally, but I'm open to any suggestions. Locally I ran yarn add --dev @types/node@14.0.4 which is a bit of a work-around, but I'm not sure on how to otherwise resolve this.

chakrihacker commented 3 years ago

Thanks for the pr @eithe appreciate all your efforts, I will test over the weekend and merge it

chakrihacker commented 3 years ago

PS2: Since apisauce uses a somewhat old version of rollup I had to resort to corresponding (and also deprecated) versions of the commonjs and node-resolve rollup plugins.

Let's try upgrading rollup to latest

eithe commented 3 years ago

Ok, great stuff. If you end up upgrading rollup in another PR, I can update this one accordingly afterwards. Just let me know :)

eithe commented 3 years ago

I had a go at upgrading packages myself, but unfortunately it was not smooth sailing. I made it compile, but couldn't get the tests to run. I'm not that familiar with ava, though, perhaps someone with more experience will spot what it is easily.

The attempt is here: https://github.com/eithe/apisauce/tree/umd-build-update-packages

And the tests fail with:

λ yarn test
yarn run v1.22.4
$ npm-run-all compile test:unit
$ tsc -p tsconfig.json
$ ava -s

  × No tests found in test\async-request-transform.test.js
  × No tests found in test\async-response-transform.test.js
  × No tests found in test\async.test.js

  ─

  Uncaught exception in test\async-request-transform.test.js

  TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be one of type string, Buffer, or URL. Received type object

  » newLoader (node_modules/pirates/lib/index.js:104:7)

... repeated for all files
chakrihacker commented 3 years ago

Thanks for trying to upgrade packages, I think upgrading packages should be on another pr, I am merging this as there are no breaking changes 🎉

eithe commented 3 years ago

Thanks for merging, but remember to have a look at the @types/node issue mentioned above before releasing this since I couldn't get apisauce to compile even before my changes.

chakrihacker commented 3 years ago

Found the issue https://rollupjs.org/guide/en/#error-this-is-undefined

infinitered-circleci commented 3 years ago

:tada: This PR is included in version 2.1.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: