-
Notifications
You must be signed in to change notification settings - Fork 14
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
feat(hub-common): add minimal definitions of ArcGIS types and drop arcgis-js-api #1797
Conversation
affects: @esri/hub-common
affects: @esri/hub-common
affects: @esri/hub-common
affects: @esri/hub-common
affects: @esri/hub-common we now see TS errors when running karma tests (which still pass)
…ependencies affects: @esri/hub-common
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me despite it not being optimal to switch to :any.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1797 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1032 1035 +3
Lines 18812 18879 +67
Branches 3428 3450 +22
=========================================
+ Hits 18812 18879 +67 ☔ View full report in Codecov by Sentry. |
# @esri/hub-common [15.30.0](https://github.com/Esri/hub.js/compare/@esri/[email protected]...@esri/[email protected]) (2025-02-07) ### Features * **hub-common:** add minimal definitions of ArcGIS types and drop arcgis-js-api ([#1797](#1797)) ([27c08b8](27c08b8))
We were using @types/arcgis-js-api, which has been deprecated and effectively stopped being published as of version 4.31.
They recommend that we use the types from
@arcgis/core
instead, and I tried that and it worked up until version 4.31, but using the 4.32 rc I see all kinds of type errors when running Karma tests (which still pass). The other downside of using@arcgis/core
types is that b/c this repo usestslint
instead ofeslint
, we can't use @typescript-eslint/no-restricted-imports to ensure that devs do not accidentally bloat the build by importing more than just the types.We only use the ArcGIS types in a couple of places, so I thought, for now we can just replace those types w/
a minimal definition of the types we need.any
Instructions for testing:
Closes Issues: # (if appropriate)
Updated meaningful TSDoc to methods including Parameters and Returns, see Documentation Guide
used semantic commit messages
[x PR title follows semantic commit format (CRITICAL if the title is not in a semantic format, the release automation will not run!)
updated
peerDependencies
as needed. CRITICAL our automated release system can not be counted on to updatepeerDependencies
so we must do it manually in our PRs when needed. See the updating peerDependencies section of the release instructions for more details.