-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Upgrade mapbox-gl #1400
base: feat#1289-simplified-aoi-creation
Are you sure you want to change the base?
fix: Upgrade mapbox-gl #1400
Conversation
✅ Deploy Preview for veda-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@AliceR I think the issue is a mismatch between the
|
Thanks @dzole0311 , this indeed fixed all the ts errors. I am still not sure if I solved all the issues correctly... And now I even wonder why we have this duplication between |
hey @AliceR 👋🏼 . Just saw your question in the description but what could I help out with specifically when it comes to typing? There is lots going on with it and much improvement in our codebase needed for it 😄 . A few things I can summarize right now though that might help out..
lmk if I could help assist in any of these areas :) |
Thank you for the context @sandrahoang686 , it makes a lot of sense! The only thing missing here then would be a review decision: can this PR be merged as is, and if not, which of the tech debt should we include? |
@@ -206,7 +206,7 @@ function MapBlock(props: MapBlockProps) { | |||
parallels: projectionParallels | |||
}); | |||
return { | |||
...projection, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spread types may only be created from object types.ts(2698)
const projection: MbProjectionOptions
const metadata: ExtendedMetadata = generatorLayer.metadata ?? {}; | ||
metadata.generatorId = generatorId; | ||
const metadata: ExtendedMetadata = { | ||
...(generatorLayer.metadata ?? {}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...(generatorLayer.metadata ?? {}), | |
...generatorLayer.metadata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, this will only work if we don't pass unknown
to the metadata?: ExtendedMetadata
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we create a tech debt ticket to clean up typings around maps/sources/metadata? This seems to be a bit of a 🧶 (alt: a knotty situation)
Co-authored-by: Gjore Milevski <[email protected]>
I am attempting to upgrade mapbox-gl, in order to use the fitBounds with alternative projections. Version 3.5 introduces breaking changes to the type system, which I was able to address in e227a9d. I have some issues left that I need help with, also need someone with better understanding of our veda typings to have a proper look at my changes.
@sandrahoang686 maybe? 😊