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

Fix typings issues in 3.2.0 #2898

Merged
merged 17 commits into from
Jul 24, 2023
Merged

Fix typings issues in 3.2.0 #2898

merged 17 commits into from
Jul 24, 2023

Conversation

neodescis
Copy link
Collaborator

@neodescis neodescis commented Jul 21, 2023

Closes #2896.

  • Fixed inferred typing of event data in map.on(), map.off() and map.once()
  • Elevated @types/supercluster to a regular dependency
  • Specified the typing for _updateHash rather than let it be inferred (since it is inferred to () => NodeJS.Timeout)

Launch Checklist

  • Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
  • Briefly describe the changes in this PR.
  • Link to related issues.
  • Include before/after visuals or gifs if this PR includes visual changes.
  • Write tests for all new functionality.
  • Document any changes to public APIs.
  • Post benchmark scores.
  • Add an entry to CHANGELOG.md under the ## main section.

package.json Show resolved Hide resolved
src/ui/map.ts Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Jul 21, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (1674322) 74.49% compared to head (8f8c537) 74.49%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2898   +/-   ##
=======================================
  Coverage   74.49%   74.49%           
=======================================
  Files         238      238           
  Lines       19018    19018           
  Branches     4285     4285           
=======================================
  Hits        14167    14167           
  Misses       4851     4851           
Impacted Files Coverage Δ
src/ui/map.ts 82.92% <ø> (ø)
src/ui/hash.ts 98.52% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

src/ui/map.ts Show resolved Hide resolved
@HarelM
Copy link
Collaborator

HarelM commented Jul 22, 2023

Overall this looks good, thanks!
Added a few minor comments.

@HarelM
Copy link
Collaborator

HarelM commented Jul 22, 2023

Also can you ping me privately on maplibre's Slack ("Harel Mazor" is my handle there)?

build/generate-typings.ts Outdated Show resolved Hide resolved
tsconfig.typegen.json Outdated Show resolved Hide resolved
@HarelM
Copy link
Collaborator

HarelM commented Jul 22, 2023

Added a few more comments, thanks for bearing with me.

tsconfig.dist.json Outdated Show resolved Hide resolved
tsconfig.json Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@HarelM HarelM left a comment

Choose a reason for hiding this comment

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

Overall approved, thanks!

@neodescis
Copy link
Collaborator Author

I've tested everything out I can think of:

  • NodeJS global types are not included in the generated .d.ts file
  • Even if I put a NodeJS global type in manually, the typecheck script fails
  • The typecheck script fails on type errors in tests, including the assertNotAny() check I've added

@HarelM HarelM merged commit 9a5af6f into maplibre:main Jul 24, 2023
@HarelM
Copy link
Collaborator

HarelM commented Jul 24, 2023

Thanks! Feel free to create a version bump PR so we can release a new version with these fixes.

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.

MapLibre 3.2.0 Typing Errors
3 participants