fix: handle zero as a valid value for marker position

[s-montigny-desautels]

Use null check to prevent 0 value to be falsy.

Also, the code new google.maps.LatLng(null); return a marker with the coordiate (NaN, 0) and it break the SuperCluster algorithm.

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

• [x] 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
• [x] Ensure the tests and linter pass
• [x] Code coverage does not decrease (if any source code was changed)
• [x] Appropriate docs were updated (if necessary)

Fixes #891 🦕

[google-cla[bot]]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[shopi-dori]

Hi @s-montigny-desautels ! 👋 Is this PR still being actively worked on? Our team is experiencing this problem as well, and I was about to submit a PR when I found this. Thanks for looking into it 🙏

To pass checks and be ready for review, it looks like this PR just needs a supporting unit test update to gets the marker position... . Something along the lines of adding coordinate values of [0, 0], [0, N], [N, 0]:

    test("gets the marker position and returns a LatLng", () => {
      // test markers created with LatLng and LatLngLiteral
      [
        new google.maps.LatLng(1, 1), { lat: 1, lng: 1 }, 
        new google.maps.LatLng(0, 0), { lat: 0, lng: 0 },
        new google.maps.LatLng(0, 1), { lat: 0, lng: 1 },
        new google.maps.LatLng(1, 0), { lat: 1, lng: 0 },
      ].forEach((position) => {
        const marker = new markerClass({ position: position });
        if (markerClass === google.maps.marker.AdvancedMarkerElement) {
          // TODO: consider getting more specific with the lat/lng checks
          (marker as google.maps.marker.AdvancedMarkerElement).position =
            position;
        }
        expect(MarkerUtils.getPosition(marker)).toBeInstanceOf(
          google.maps.LatLng
        );
      });
    });

Is this something I can help with, or would you prefer to add it?

[s-montigny-desautels]

Hi, thanks for the info, I didn't know I needed to add a new test for this.

I can add those tests, but it's kind of pointless, since we can't actually test that the Lat & Lng is set to 0, since google.maps.LatLng class is mocked... Do you have a proposal for this?

[shopi-dori]

I can add those tests, but it's kind of pointless, since we can't actually test that the Lat & Lng is set to 0, since google.maps.LatLng class is mocked... Do you have a proposal for this?

@s-montigny-desautels 🤔 I can't think of anything right this moment, but let me try out some things when I'm back in the office next week.

@wangela it looks like you're most active on PRs for this repo. Would you happen to have a suggestion for the unit tests or someone we could reach out to for help? (I didn't immediately see anything in the Contribution guide.)

[shopi-dori]

@s-montigny-desautels Sorry for the delay. Here's a draft PR with the approach I was thinking about for the solution and testing--needs work, but hopefully this conveys the idea: https://github.com/googlemaps/js-markerclusterer/pull/927/files

To run the tests, execute at the command line: npx jest src/marker-utils.test.ts (ignore npx if you have jest installed globally)

The LatLng tests pass, but the LatLngLiteral tests are failing. This might be related to the mocking you were mentioning, but I'd need to familiarize myself with @googlemaps/jest-mocks to troubleshoot more. If there's a way to get some feedback from a repo owner, that would definitely be a faster path to a solution, but I'm not sure who to reach out to based on the contributor guidelines.

In the mean time, our workaround is to intercept values of 0 for lat or lng and convert those to 0.00000001, which appears to round to 0 in the clusterer. It sounds like 6-7 decimal places may be the max (see ref), but I still need to confirm this as well.