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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add loading=async to the urls with callbacks #822

Merged
merged 2 commits into from
Feb 15, 2024

Conversation

nora-soderlund
Copy link
Contributor

This PR adds loading=async as a combination with the callback parameter to resolve the warning message.

From the Discord:

hello, do you guys know how to get rid off the warning below while using APIProvider component from '@vis.gl/react-google-maps' ? I don't see there any flag to set regarding loading type, obviously I could load the maps in old vanilla JS way but it's not a point while I have installed the library package 馃槮

"Google Maps JavaScript API has been loaded directly without loading=async. This can result in suboptimal performance. For best-practice loading patterns please see https://goo.gle/js-api-loading"

From the documentation:

loading: The code loading strategy that the Maps JavaScript API can use. Set to async to indicate that the Maps JavaScript API has not been loaded synchronously and that no JavaScript code is triggered by the script's load event. It is highly recommended to set this to async whenever possible, for improved performance. (Use the callback parameter instead to perform actions when the Maps JavaScript API is available.) Available starting with version 3.55.

Thank you for opening a Pull Request!


Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open a GitHub issue as a bug/feature request before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> 馃

@usefulthink
Copy link
Contributor

Looks good to me, but eslint didn't like something about the formatting. Can you run npm run format and push again?

@nora-soderlund
Copy link
Contributor Author

Yeah, sorry, did this over the web. I've fixed the linting @usefulthink :)

@nora-soderlund
Copy link
Contributor Author

I just noticed that this is the wrong library 馃槃 I mistakenly thought that the @vis.gl/react-google-maps package uses this repository but it has its own loader.

I guess this is still a valid fix? 馃槤

@wangela
Copy link
Member

wangela commented Feb 15, 2024

Thanks for this fix, @nora-soderlund! Yup, even though this isn't a fix for vis.gl/react-google-maps this is a good fix to incorporate into the js-api-loader.

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.

None yet

3 participants