Conversation
metalwarrior665
left a comment
There was a problem hiding this comment.
Thanks, looks good and clean. Please go through the comments and either implement the proposed changes or argue for different solution :)
| * | ||
| * Returns `undefined` when `rawDate` is missing or invalid. | ||
| */ | ||
| export function rawToLocalISOString(rawDate: string | undefined, endOfDay = false): string | undefined { |
There was a problem hiding this comment.
You should better explain to both users and readers of the code but you are approaching the event times. As I understand it, you get the event time in the local timezone since that is really the only relevant time for the event.
I didn't really understand the full time conversion logic you are doing but for filtering event time with since and till, I think it is easier to take the current UTC time and then convert each event time to UTC time (since each event can technically have different timezone, right?). Then you just work in UTC which is the standard.
| } | ||
|
|
||
| const nextPage = request.userData.page + 1; | ||
| await addRequests([ |
There was a problem hiding this comment.
- There is no way to know if there any more events on the next page? Currently, you have to do one more request to see if there are 0 results. You should be able to count the current events on the page or maybe there is some total events indiciation in the response.
- If you would know the total, you could theoretically enqueue all the pages upfront here to speed up scraping. But it is probably already very fast.
There was a problem hiding this comment.
- I haven't found any indication.
- Yes, makes sense, but I don't know them upfront. It is really fast either way 😄
https://apify.com/emdzej/tootoot-actor