Closed joshmossas closed 2 years ago
Also I'm not sure if I need to include the changes in ./lib
and ./dist
. Is this handled by a build system? Or should I included the newly outputted files in this PR?
Thank you @joshmossas! 🙏
The lib
and dist
directories get built as part of the build/publishing process, so we can skip that as part of this PR.
Could you also add a test for this new method similar to the one here: https://github.com/typesense/typesense-js/blob/b1d7ca1c930d09b95e55b44e4350d2b178d3aaf1/test/Typesense/Documents.spec.js#L376-L401
Hey @jasonbosco
I've added a test for .exportStream as requested.
./test-files/exportStreamData.jsonl
and then deleted after the test completes. I've added ./test-files
to the gitignore. However I can use a different location if you prefer.directoryExists
and getDataFromStream
because I was running into a conflict with Prettier and the Eslint rule space-before-function-paren
. Prettier would format the function to remove the space between the function name and the space (Basically it would turn function myFun ()
to function myFunc()
) resulting in a linting error. Because of this I used the arrow syntax for these two methods to get around it. I only made note of this since the rest of the file uses the regular function syntax.LGTM
Published this in v1.0.3-3
@jasonbosco Just so you know, the exportStream method is still not present in v1.0.3-3
. Perhaps there was an error in the build/publishing process?
My bad. I just published 1.0.3-4
with the latest build.
No worries. Thanks my dude.
Change Summary
This pull request closes #87
Changes to
ApiCall.ts
performRequest()
now acceptsresponseType
as an optional parameter. That parameter is passed to Axios when making a request.get()
now acceptsresponseType
in it's optional parametersChanges to
Documents.ts
exportStream
. It's functionally the same asexport()
the only difference is it now it passes{ responseType: "stream" }
to thethis.apiCall.get()
.Additional Notes
Axios doesn't support returning a stream in the browser. So if this is called in the browser it'll just return a string like normal. Not sure if this needs to be documented or not. We could add a note to the JSDoc comment although I imagine most people wouldn't be using the export methods in the browser anyway since they require admin keys.
All tests are passing. I also did some basic tests in NodeJS and the Browser.
PR Checklist