Skip to content
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

Add load prop to Map #28

Merged
merged 1 commit into from
Mar 26, 2023
Merged

Add load prop to Map #28

merged 1 commit into from
Mar 26, 2023

Conversation

derekr
Copy link
Contributor

@derekr derekr commented Mar 22, 2023

tl;dr

// same interface as the internal util/loader module
<Map load={() => new Promise(...)} />

This enables the caller to have more control over how and when the mapkit resources are loaded. Instead of supporting more API sruface area in mapkit-react library I thought it made sense to just hand over control at the load point as a "advanced use" option. This could lead to foot-guns if the caller doesn't load the necessary resources and then tries to use a component from the lib that depends on the missing resource. In this case the error messages should be clear enough (mapkit says the resrouce needs to be loaded when using associated APIs).

Needed this flexibility to support my own desires for optimizing map loading performance https://developer.apple.com/documentation/mapkitjs/loading_the_latest_version_of_mapkit_js#3331749.

My main driver was wanting to optimize map loading as much as possible rendering my own static script tag and using async and fetchpriority attributes which aren't possible when loading the script on demand.

This enables the caller to have more control over how and when the mapkit resources are loaded. Instead of supporting more API sruface area in mapkit-react library I thought it made sense to just hand over control at the load point as a "advanced use" option. This could lead to foot-guns if the caller doesn't load the necessary resources and then tries to use a component from the lib that depends on the missing resource. In this case the error messages should be clear enough (mapkit says the resrouce needs to be loaded when using associated APIs). Needed this flexibility to support my own desires for optimizing map loading performance https://developer.apple.com/documentation/mapkitjs/loading_the_latest_version_of_mapkit_js#3331749.
Copy link
Owner

@Nicolapps Nicolapps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a great addition, thank you for the PR!

@Nicolapps Nicolapps merged commit f73f3bb into Nicolapps:main Mar 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants