fix: Mark underscored methods as private

[awesomephant]

I took a stab at #186. The approach is to mark all underscored methods as private (where not already the case), which excludes them from the docs:

However, this breaks a bunch of unit tests that want to spyOn() those methods. I addressed that by switching some method calls to bracket notation, ie:

- geocoder._renderError();
+ geocoder["_renderError"]();

... and bumping vitest and vitest/coverage-v8 to ^v5.0.0-beta.2, which adds support for spying on private methods.

It's unclear to me if marking these methods private constitutes a breaking change. Happy to amend the PR when/if the vitest feature makes it into a stable release.

Closes #186

---

• briefly describe the changes in this PR
• update CHANGELOG.md with changes under main heading before merging

[HarelM]

Generally speaking, spying on private methods is a code smell, unit tests should relay on the public API of a class only. But there's a lot of history in this repo, so it's not an easy fix.
We haven't released version 6 yet, so we might consider adding this as a "breaking change" ,although it's not really a breaking change, but version 6 is possibly a good candidate for this change.
I think the main problem is about internal use inside this library of "private" (underscore) methods, not even in unit tests, just places in production code.
To be fair, I think this issue was opened a long time ago and I'm not sure it's worth it (although I advocated for this change), but if you would like to drive this through I'll be happy to review these kind of changes, but it might be smarter to split those changes to small PRs for easy review...
Let me know what you think.

[awesomephant]

but it might be smarter to split those changes to small PRs for easy review

As in, one PR would mark one method as private, removing/rewriting tests that rely on it?

I think the main problem is about internal use inside this library of "private" (underscore) methods, not even in unit tests, just places in production code.

can you expand a bit on what a better approach would be?

[HarelM]

I'm not sure I have a better approach besides removing _ from methods/fields that are in use outside a class (like _map, _style, etc).
But I'm not sure that's a good solution to be honest...

One PR for a class/file I think would make sense.

[awesomephant]

Makes sense. Feel free to close this PR then (if you see fit) and I'll see if I can chip away at this

[HarelM]

Oh, wait, I was under the impression we are talking about maplibre-gl-js, my mistake.
In this repo it should be a lot easier.
Just make the unit test use the public API and we can release a breaking change version to be on the safe side.
Sorry...

[codecov]

Codecov Report

❌ Patch coverage is 0% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.00%. Comparing base (9300df5) to head (fb2d392).
⚠️ Report is 107 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/index.ts	0.00%	15 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (9300df5) and HEAD (fb2d392). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (9300df5)	HEAD (fb2d392)
	2	1

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #638       +/-   ##
==========================================
- Coverage   88.80%   0.00%   -88.81%     
==========================================
  Files           3       3               
  Lines         929     561      -368     
  Branches      189     154       -35     
==========================================
- Hits          825       0      -825     
- Misses        104     561      +457     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.