Techeron / fvlr_api

0 stars 2 forks source link

Change scrapers return #9

Open honjes opened 2 months ago

honjes commented 2 months ago

I would like to change the return of scrapers.

Currently, the return Object is defined beforehand and gets assigned values to it like below

const event = {} as Event
event.link = `https://www.vlr.gg/event/${id}`
...

This has the problem that when you add a property to the Event type, there is no error that is missing. For example, if status would get added to the event type, there is no error in typescript.

I propose to change it to the following:

const event: Event = {
  link: `https://www.vlr.gg/event/${id}`
  ...
}

Now, when there is a change to the type that does not match the return, there will be an error thrown.

What do you think ?

Codokk commented 2 months ago

The reason it is done the way it is right now is because of all the scraping logic taking up so much of the page. We need to either define a ton of variables and then at the end put them into the event const or we need to create a temporary object to hold all this data and do it at the end anyways.

This comes down to preference and if we think that the lack of typing support is worth the duplication of code

honjes commented 2 months ago

I would argue that the type support is worth it because of errors that could be caught before going into the code. And also an easier time adding return data, in my opinion, because you can edit the schema first and then just "fix" the given error.

But I can also see that a ton of vars would be created and would add some more code to the functions. So it's your call. i would love to do it, just because I'm a huge fan of support all the way.

Codokk commented 2 months ago

Sounds good to me