[Geo I] Typing functionality #5179
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const output = OUT_PATH.split('/').pop(); | ||
| console.info(`Generating new ${output}`); | ||
| const topo = topology({ | ||
| objects: fc, | ||
| }); | ||
|
|
||
| // We do the following to match the specific format needed for visualization | ||
| const objects: any = topo.objects.objects; | ||
| const newObjects = {}; | ||
| const objects = topo.objects.objects as any; |
Check warning
Code scanning / ESLint
Disallow the `any` type
✅ Deploy Preview for phenomenal-syrniki-c5ceaa ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
madsnedergaard
changed the title
web/scripts/generateZonesConfig.ts
Outdated
| @@ -124,7 +125,7 @@ const mergeRatioParameters = () => { | |||
| return ratioParameters; | |||
| }; | |||
|
|
|||
| const writeJSON = (fileName: any, object: any) => { | |||
| const writeJSON = (fileName: string, object: unknown) => { | |||
There was a problem hiding this comment.
We should try and define the object type here later on but I relize it's not the simplest of types 😅
There was a problem hiding this comment.
Yeeeeeeah, I started doing that, but it becomes a type of anything that can be JSON.stringify'ed, and when looking in the native NodeJS types and around on the interwebs it seems like this was most common practice. And given the actual impact of this, I did not consider it worth pursuing further - but I'm very open to changing my mind on this 😄
There was a problem hiding this comment.
Since we are only writing 4 kinds of JSON objects (at least right now) we could make 4 types and then a union type for this object.
These would be zone, world, exchanges and the aggregated exchanges thing.
We should already have types for the zone at least right?
There was a problem hiding this comment.
Hmm, I started doing this, but now that I think of it I'm not sure we should limit what can be written to a JSON file - it feels counter-intuitive to be typing what output the "excluded exchanges" should be in here, when this function really shouldn't care about this 🤔 Happy to revisit this later though!
There was a problem hiding this comment.
Makes sense, and in any case the type errors should have been caught before we write things to json.
Thanks for reviewing - and I found it really hard with TS to split it up further, as changes to one functions types affects other places. I attempted to split it up a bit by commits though, but would love to hear your thoughts on how to make reviewing easy next time :) |
|
We could make some improvements but the output is the same as it used to be and having typing will be extra nice when we want to make changes in the future! |
| subZoneNames?: string[]; | ||
| bounding_box: number[][]; | ||
| timezone: string; | ||
| [key: string]: any; |
Check warning
Code scanning / ESLint
Disallow the `any` type
| capacity?: [number, number]; | ||
| lonlat: [number, number]; | ||
| rotation: number; | ||
| [key: string]: any; |
Check warning
Code scanning / ESLint
Disallow the `any` type
Description
First step in improving the geometry-related code we have: Typing it up.
I have a few more PRs coming up as I'm splitting up the work I've done :)