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

TypeScript types file #319

Closed
gavinr-maps opened this issue Sep 4, 2024 · 11 comments · Fixed by #321
Closed

TypeScript types file #319

gavinr-maps opened this issue Sep 4, 2024 · 11 comments · Fixed by #321

Comments

@gavinr-maps
Copy link
Contributor

gavinr-maps commented Sep 4, 2024

Problem

As a follow-up from #259 and #318 (comment), it seems like the TypeScript types at https://www.npmjs.com/package/@types/esri-leaflet are ok but not working in certain situations.

Solution

We should publish our own typescript types file within this repo, like we did within Esri/esri-leaflet-vector#114

Replication Steps

  1. Run the following in a terminal:
npm create vite@latest test-esri-leaflet-geocoder -- --template vanilla-ts
cd test-esri-leaflet-geocoder
npm install

npm install leaflet
npm i --save-dev @types/leaflet

npm install esri-leaflet-vector
npm install esri-leaflet-geocoder

npm run dev
  1. add the following to the top of src/style.css:
@import url(https://unpkg.com/[email protected]/dist/leaflet.css);
@import url(https://unpkg.com/esri-leaflet-geocoder/dist/esri-leaflet-geocoder.css);

#app {
  width: 800px;
  height: 400px;
}

REPLACE the file src/main.ts with this:

import "./style.css";

import * as L from "leaflet";
// import * as esriLeaflet from "esri-leaflet";
import * as esriLeafletVector from "esri-leaflet-vector";
import * as esriLeafletGeocoder from "esri-leaflet-geocoder";

const createMap = (domNode: HTMLDivElement) => {
  const apikey =
    "YOUR_API_KEY";
  var map = L.map(domNode).setView([45.5165, -122.6764], 12);
  // esriLeaflet.basemapLayer("Streets").addTo(map);
  esriLeafletVector
    .vectorBasemapLayer("ArcGIS:Streets", {
      apikey,
    })
    .addTo(map);

  // create the geocoding control and add it to the map
  var searchControl = esriLeafletGeocoder
    .geosearch({
      providers: [
        esriLeafletGeocoder.arcgisOnlineProvider({
          // API Key to be passed to the ArcGIS Online Geocoding Service
          apikey,
        }),
      ],
    })
    .addTo(map);

  // create an empty layer group to store the results and add it to the map
  var results = L.layerGroup().addTo(map);

  // listen for the results event and add every result to the map
  searchControl.on("results", function (data: any) {
    results.clearLayers();
    for (var i = data.results.length - 1; i >= 0; i--) {
      results.addLayer(L.marker(data.results[i].latlng));
    }
  });
};
createMap(document.querySelector<HTMLDivElement>("#app")!);
  1. Open the project in VS Code.
  • Expected: No Typescript errors.
  • Actual: Could not find a declaration file for module 'esri-leaflet-geocoder'. 'c:/../node_modules/esri-leaflet-geocoder/dist/esri-leaflet-geocoder-debug.js' implicitly has an 'any' type. Try npm i --save-dev @types/esri-leaflet-geocoder if it exists or add a new declaration (.d.ts) file containing declare module 'esri-leaflet-geocoder';ts(7016)
@nooras
Copy link
Contributor

nooras commented Oct 5, 2024

Hey @gavinr
I am interested in this task. Can you please assign me ?
Also which specific type need to be added ?

@hhkaos
Copy link
Member

hhkaos commented Oct 14, 2024

Sorry @nooras I just found out that @gavinr was out of the office last week. Let me ping @patrickarlt & @cyatteau

@gavinr-maps
Copy link
Contributor Author

I am interested in this task. Can you please assign me ?

It is not necessary for you to be assigned in order for you to work on this task - please feel to go ahead and work on it! Just fork the repo, create a branch, make the changes in that branch, and then send a PR back to this repo.

Also which specific type need to be added ?

I think it would be types for any of the pubic APIs, i.e. what is listed here: https://github.com/Esri/esri-leaflet-geocoder?tab=readme-ov-file#api-reference

@kimmykokonut
Copy link

Is @nooras assigned to this? I don't want to pick up an issue if someone is already on it.

@nooras
Copy link
Contributor

nooras commented Oct 18, 2024

Is @nooras assigned to this? I don't want to pick up an issue if someone is already on it.

Hey @kimmykokonut I am working on it. Thanks

@nooras
Copy link
Contributor

nooras commented Oct 20, 2024

hey @gavinr-maps, @hhkaos
I am done with changes in local. how can i verify these changes are working fine in local ?

nooras added a commit to nooras/esri-leaflet-geocoder that referenced this issue Oct 20, 2024
@nooras
Copy link
Contributor

nooras commented Oct 20, 2024

@gavinr-maps
Please review the PR. Let me know if any changes required

@hhkaos
Copy link
Member

hhkaos commented Oct 21, 2024

I'm trying to test, but I'm not as familiar as I should be with TypeScript these days.... So I'm going to defer to @gavinr-maps 😅

@gavinr-maps
Copy link
Contributor Author

@hhkaos I have added replication steps to the original issue above.

@hhkaos

This comment was marked as outdated.

@hhkaos
Copy link
Member

hhkaos commented Oct 22, 2024

@nooras, just FYI, I just checked with @gavinr-maps on a call, and the issue on my side was that I had @types/esri-leaflet-geocoder installed globally; that's why I didn't have that issue 😅. So you can proceed with his instructions

Thanks!

nooras added a commit to nooras/esri-leaflet-geocoder that referenced this issue Oct 23, 2024
hhkaos added a commit that referenced this issue Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants