search

edemaine / comingle

Multiroom meeting app integrating web tools
MIT License
44 stars 8 forks source link

Date objects vs plain JSON for API calls #155

Closed mehtank closed 3 years ago

mehtank commented 3 years ago

https://github.com/edemaine/comingle/blob/a1b49f9111c95592c7513ba2df03d79c20ad0c7c/server/log.coffee#L55-L56

I tried calling the API with a timestamp (which is what we see if we parse /api/log/get as a regular JSON), an ISO 8601 string, and a couple others, but they all resulted in Match Errors.

I realize now that you mention Date objects explicitly in the docs for why to use EJSON instead of plain JSON, but it would be nice to be more universal (and allow GET calls too). Perhaps a custom match where we feed the values into a Date constructor, and if it is successful use that result? That'll allow all manner of reasonable inputs to the api call.

edemaine commented 3 years ago

Please give the master branch a try. It should support most forms of ISO 8601, as given by the ECMAScript standard.

mehtank commented 3 years ago

The check only happens on GET parameters [ which works fine ], but not on options passed in through a POST request body:

https://github.com/edemaine/comingle/blob/2d5abfec392bd16c2f0ce95fad99e4fba9be48e1/server/api.coffee#L124

So a POST request with plain JSON still fails. That said, I now realize that a Date object is none other than a regular JSON object with key "$date" and value milliseconds-since-epoch. And that's easy enough to manually create in code, so I'm fine with it as it stands.

edemaine commented 3 years ago

Good point. I added support in the post body as well, and documented the date (and RegExp) format.