yumauri / gotenberg-js-client

A simple JS/TS client for interacting with a Gotenberg API
MIT License
111 stars 9 forks source link

passing tuples to office conversion doesn't work #11

Closed thib3113 closed 4 years ago

thib3113 commented 4 years ago

On lib version 0.5.0 .

I use function like that :

const a = pipe(gotenberg(process.env.GOTENBERG), convert, office, please)

if I pass a tuples, for example :

a(['aaaa', <readableStream>]);

I got an error :

Error: There are duplicates in file names: index.html

the problem is : here

the variable tuples contain an array :

[
  [
    "index.html",
    "aaaa"
  ],
  [
    "index.html",
    <readableStream>
  ]
]

so, here , we found some duplicates .


For informations, here,

request.source = [
  "aaaa",
  <readableStream>
  }
]
thib3113 commented 4 years ago

Seems logic, because I pass an incorrect fileName .

But I thinks the error throwed is not really the real error, and can hide some others error

Example, if two strings are passed, did you set the filename to index.html two times ? and so, throw an error because duplicate filename ?

Else, because it seems that passing two string, and the first is not a fileName, is not supported, maybe throw an error before the duplicate check

yumauri commented 4 years ago

Yeah, there is a check of first argument, if it is filename, here. Filename should match regular expression /^[\w\s\(\),-]+\.[A-Za-z0-9]+$/

But I agree, that error shown points to different direction. I'll check it, thank you!

yumauri commented 4 years ago

I've added few more source checks, published version 0.6.0

There shouldn't be any breaking changes, but it turned out there are number of commits since last release, so I decided to bump minor version, just in case :)

New checks should catch cases like:

['aaaaa', 'test']
{ aaaaa: 'test' }

and nested too:

[['aaaaa', 'test']]
[{ aaaaa: 'test' }]
[['aaaaa', 'test'], { aaaaa: 'test' }]

(Error will be thrown only on first item)

yumauri commented 4 years ago

@thib3113 do you think this is enough?

I thought about adding link to Source documentation in error message, but decided against, this looks like React-like overkill to me :)

thib3113 commented 4 years ago

yes I think . Next time I got this error, I will faster know I passed an incorrect filename xD, insteed of searching why my file is named index.html ;) .

thank you

yumauri commented 4 years ago

Cool :) Closing this then