search

purescript-contrib / purescript-affjax

An asynchronous AJAX library built using Aff.
Apache License 2.0
121 stars 78 forks source link

timeout setting for Request #151

Closed chekoopa closed 3 years ago

chekoopa commented 3 years ago

A simple timeout feature with Maybe field in Request. Test case included. Closes #143.

It was easier than I've thought.

chekoopa commented 3 years ago

Had to force-push twice because of JS formatting fixes. Srry. But still has strange issues with foreign code, however the test has been successfully completed on the local machine with Spago 0.16.0 and Purs 0.13.8.

thomashoneyman commented 3 years ago

@chekoopa Libraries in the purescript-contrib organization only support ES5 in the FFI to ensure libraries can be used without a bundling step. I'm not sure if that's specifically the error you're seeing, but I do see const and let in there, which will parse with 0.13.8 but isn't ES5.

Do you mind switching over to use ES5 in the FFI file? Most likely that will also resolve the parse issue with the foreign JS as well. In this case it mostly looks like that comes down to swapping var for const / let.

chekoopa commented 3 years ago

@thomashoneyman Sure, done that, let's see what Travis would say.

thomashoneyman commented 3 years ago

Checked the documentation and looks like setting 0 is the correct setting for "no timeout", so the Maybe encoding looks appropriate.

chekoopa commented 3 years ago

Checked the documentation and looks like setting 0 is the correct setting for "no timeout", so the Maybe encoding looks appropriate.

Yes, I've done it first without Maybe, but '0 ms' as a default value may create some misconceptions.

garyb commented 3 years ago

I think this should have been released as a breaking change - adding the field to Request means it will break for anyone who is constructing the value manually rather than overriding defaultRequest.

thomashoneyman commented 3 years ago

🤕 don't release late at night. I've updated the release to be v11.0.0 as this didn't yet make it into Spago and it's only been a few hours.