-
Notifications
You must be signed in to change notification settings - Fork 9
fix: MapBlock now expects pre-prepared data layers #94
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
Conversation
✅ Deploy Preview for veda-ui-next-test ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
vgeorge
left a comment
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.
@dzole0311 this is functional, but I wonder if we can avoid disabling type checks. The motivation seems to be that the Veda UI library doesn’t export the type definitions used here, but I think we should copy them into this repo to stay on the type safe side. I know this could bring maintainability issues, but we can create a ticket upstream to increase the exported types coverage.
|
@vgeorge I think we can’t copy the full type set because some of the types use enums and TS is stricter with enums. If you declare the same enum in two places, TS won’t let you use one where the other is expected (eg. you would get an error like I made a minimal local copy of the two needed types + linked a tracking issue and removed the |
vgeorge
left a comment
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.
@dzole0311 thanks for opening the upstream ticket. This looks good to merge 🙂
This PR updates the
EnhancedMapBlockin next-veda-ui to align with recent veda-ui changes (#1808). Instead of passing full datasets into MapBlock we now reconcile and prepare baseDataLayer (and optional compareDataLayer) before rendering.
Test instructions: