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

Error google compile #349

Open
Marius-Romanus opened this issue Oct 5, 2022 · 20 comments
Open

Error google compile #349

Marius-Romanus opened this issue Oct 5, 2022 · 20 comments

Comments

@Marius-Romanus
Copy link

Hi!

In the new version 3.7.0 it gives me a compilation error:

Captura de pantalla 2022-10-05 202917

not sure if it has to do with this PR: #311

I am using node v18

Thank you very much for the great work you do!

@kstratis
Copy link
Collaborator

kstratis commented Oct 5, 2022

Hi @Marius-Romanus !
This release pulls some additional external dependancies which you might be missing.

Could you plz run npm install and report back?

Thanks!

@Marius-Romanus
Copy link
Author

Hello, yes, I already did it before posting the issue, I also tried to delete all node_modules and package-lock.json in case something was wrong, and it kept giving me the error.

@kstratis
Copy link
Collaborator

kstratis commented Oct 6, 2022

Please try deleting node_modules, force checkout develop branch and run npm ci. Let me know how it goes.

@Marius-Romanus
Copy link
Author

Sorry I misunderstood you, do you want my project to force in the packages to "leaflet-geosearch": "https://github.com/smeijer/leaflet-geosearch#develop".
Or do you mean clone your project and go to develop?

The first case I can't install develop, it gives me an error I don't know why.

In the second case the error I get is this:

Captura de pantalla 2022-10-06 124606

Captura de pantalla 2022-10-06 123914

@kstratis
Copy link
Collaborator

kstratis commented Oct 6, 2022

I think we got lost in the translation here. 🙂
You can either use leaflet-geosearch directly into your own project as a dependancy or crack it open and change it/modify it in any way you see fit.

Judging from your screenshots it looks like you're going for option A which means you're trying to use it on your own project but for some reason a couple of references fail to resolve when you npm install.

Could you plz share a bit more about your environment?
Looks like you're using Windows, but could you be more specific?
You mentioned node 18, but which npm version are you using? (Powershell/bash/Chocolatey/WSL)?

Thanks!

@Marius-Romanus
Copy link
Author

I'm sorry, i'm terrible at English and git... 🤦‍♂️😅
When I put the issue I probed with Node 18.4.0, but now I have also tested with version 18.10.0.
The operating system is Windows 10 Pro.
npm version 8.19.2.
Chocolatey v1.1.0

Later I will create a project from 0 to test, in case it was something uniquely mine. I'll keep you informed.

If this happens to someone else and reads it, let me know if it's just me and close the issue or if it's generalized.

@Marius-Romanus
Copy link
Author

Hi, I have created a new project in Angular, installed the latest version of leaflet-geosearch and simply imported { OpenStreetMapProvider } from 'leaflet-geosearch'; in app.component.ts and it gave me the same error that I mentioned at the beginning.

I've created a git if you want to take a look at it: https://github.com/Marius-Romanus/bug-leaflet-geosearch

@darrenklein
Copy link
Collaborator

darrenklein commented Oct 7, 2022

@Marius-Romanus @kstratis I wonder if this is an issue with the API key configuration? It might be that the user needs to configure their key for the JavaScript API, or else the client script won't load and might be raising these errors?

@darrenklein
Copy link
Collaborator

darrenklein commented Oct 7, 2022

Oh wait nevermind, these are compilation errors, sorry, got ahead of myself there.

@kstratis
Copy link
Collaborator

Looks like you're missing a few types.
Please add the following to your package.json under devDependencies and run npm install again:

"@types/google.maps": "^3.50.2",
"@types/leaflet": "^1.8.0"

Not really sure but this could be a side-effect of d58c363.

I'm wondering if we can mitigate this somehow or at least document it.

@Marius-Romanus
Copy link
Author

With types it works, although I don't think it's ideal if you don't need to be so restrictive and change the leaflet objects

@kstratis
Copy link
Collaborator

I know it's not. For now though it's the best I can come up with.

Perhaps @smeijer could share some further insight...

@smeijer
Copy link
Owner

smeijer commented Oct 10, 2022

@Marius-Romanus , I agree it's not ideal, but it's how typescript works. You're importing from the root, and thereby checking types in this project. For that the google types have to be included.

Instead of importing the provider from the root, it's also possible to import it via a direct import. That way, you'll only import the specific provider that you need, and will not run into the missing types error. Depending on your build stack, that might also result in a smaller bundle size. So it's worth it regardless of the error.

import { Component } from '@angular/core';
import OpenStreetMapProvider from 'leaflet-geosearch/lib/providers/openStreetMapProvider';

const provider = new OpenStreetMapProvider();

@Component({
  selector: 'app-root',
  templateUrl: './app.component.html',
  styleUrls: ['./app.component.scss']
})
export class AppComponent {
  title = 'demo-leaflet-geosearch';
}

If you'll need the SearchControl as well, because you're using the leaflet search bar, you can do so like:

import GeoSearchControl from 'leaflet-geosearch/lib/SearchControl';
import OpenStreetMapProvider from 'leaflet-geosearch/lib/providers/openStreetMapProvider';

const provider = new OpenStreetMapProvider();

new GeoSearchControl({
  provider,
}).addTo(map);

@Marius-Romanus
Copy link
Author

Hello, yes I agree, I always try to follow the types of typescript and it works for me, I only said it for other users. If you consider it you can close the issue. Thank you all very much for the project. :)

@smeijer
Copy link
Owner

smeijer commented Oct 10, 2022

Not really sure but this could be a side-effect of d58c363.

I'm wondering if we can mitigate this somehow or at least document it.

It definitely is a side-effect of that pull request. I see three options:

  1. Add the workaround I mentioned above to the readme. If we don't change the code, that should be done. And honestly, it's a good convention to have documented anyways, for smaller bundle sizes.

  2. Don't re-export all the providers from src/index. Even with the documentation of 1), people will still run into the errors. By not re-exporting everything from the root, we force people to import from paths. This does make things more complicated for those that use the CDN version tho.

3. Don't import the types from @types/google.maps, but include the limited set of types we need directly in the provider file, like we do with other providers. We don't need complete coverage, we just need to cover the "public interface".

The last one isn't going to work, as @googlemaps/js-api-loader also depends on @types/google.maps.

@smeijer
Copy link
Owner

smeijer commented Oct 10, 2022

This is the typedef that we should include in googleProvider.ts if we want to go that route. If we do, we still need to refactor out the js-api-loader dependency:

declare namespace google.maps {
  interface GeocoderAddressComponent {
    long_name: string;
    short_name: string;
    types: string[];
  }

  interface LatLngBoundsLiteral {
    east: number;
    north: number;
    south: number;
    west: number;
  }

  interface LatLngLiteral {
    lat: number;
    lng: number;
  }

  interface GeocoderGeometry {
    location: { toJSON(): google.maps.LatLngLiteral };
    viewport: { toJSON(): google.maps.LatLngBoundsLiteral };
  }

  interface GeocoderResult {
    address_components: google.maps.GeocoderAddressComponent[];
    formatted_address: string;
    geometry: google.maps.GeocoderGeometry;
    partial_match?: boolean;
    place_id: string;
    plus_code?: unknown;//google.maps.places.PlacePlusCode;
    postcode_localities?: string[];
    types: string[];
  }

  enum GeocoderStatus {
    ERROR = 'ERROR',
    OK = 'OK',
    ZERO_RESULTS = 'ZERO_RESULTS',
  }

  class Geocoder {
    geocode(
      request: { address?: null | string },
      callback?:
        ((a: google.maps.GeocoderResult[]|null,
          b: google.maps.GeocoderStatus) => void)|
        null): Promise<{ results: google.maps.GeocoderResult[] }>;
  }
}

@kstratis
Copy link
Collaborator

Is there any way we can "force" the library pull in these 2 automatically?

"@types/google.maps": "^3.50.2",
"@types/leaflet": "^1.8.0"

I mean if the user doesn't have to do the extra npm install step we should be good.
Since these are just types they probably won't end up in the user's final bundle, right?

@ivan-price-acted
Copy link

ivan-price-acted commented Nov 23, 2023

Hi guys, just chiming in with my 2c.. i was indeed required to include @types/google.maps in my devDependencies, it seems this is a hard requirement for a new out-of-the-box installation.

@smeijer 's suggestion of importing directly to avoid the root import did not work for me.

I was also required to declare GeoSearchControl as any to get the compiler to be happy:

const searchControl = new (GeoSearchControl as any)({
  provider: provider,
  style: 'bar',
})

to avoid the error:
error TS7009: 'new' expression, whose target lacks a construct signature, implicitly has an 'any' type.

In my package.json i have in devDependencies:

    "@types/leaflet-geosearch": "^3.0.0",
    "@types/google.maps": "^3.50.2",

and in dependencies:

    "leaflet-geosearch": "^3.11.0",

Just leaving this here for future travellers, maybe something should be added to the README because i don't think adding types to devDependencies should be seen as a 'bad' thing, as long as they don't end up in the bundle i wouldn't spend energy refactoring.

Thanks for the great library :)

-ivan

@augusthjerrild
Copy link

Hello.

I was facing the same issue, and therefore I found this thread :-)

I assume the google package and it's dependencies is only for the cases, where you wanna use google instead of the OpenStreetMap.

I'm really not interested in installing the google maps package, if I don't have any use of it. Is it possible to use this geosearch lib, without needing the google maps types?

Kind regards

@Marius-Romanus
Copy link
Author

Hello, you don't have to install the Google library, you just have to install the types in development: npm i -D @types/google.maps 🙂

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

No branches or pull requests

6 participants