ol-geocoder icon indicating copy to clipboard operation
ol-geocoder copied to clipboard

Published 20 hours ago • verified publisher icon Dominique92

Broken on Openlayers 7.1.0

Open geraldo opened this issue 2 years ago • 17 comments

Since updating to latest OL 7.1.0 i get the following error in the console:

Uncaught TypeError: class constructors must be invoked with 'new'

geraldo avatar Sep 09 '22 09:09 geraldo

Yea getting the same error and Geocoder is not a constructor

fogracvxy avatar Sep 09 '22 17:09 fogracvxy

Same thing happened with ol-layerswitcher. The solution seems to be removing the webpack transpiling ES6 classes to ES5. Would be great if anyone can make a PR. Can take a look at this commit for reference.

kirtan-desai avatar Sep 10 '22 14:09 kirtan-desai

Workaround: Use ol-geocoder-debug.js and replace line 1134 by: return new Control({ element: this.container });

Dominique92 avatar Sep 17 '22 15:09 Dominique92

I am working on the issue. The Base class in base.js calls super after accessing "this" which throws an error in ES6. Trying to refactor that.

EDIT: Done with this. ol-geocoder will now ship with ES6 classes. Will make a PR. Only issue I'm facing now is that the magnifying glass is not visible on the button since OL7 has default white background for colors. Will try to change color of magnifying glass.

kirtan-desai avatar Sep 17 '22 16:09 kirtan-desai

Something in there ? https://stackoverflow.com/questions/51860043/javascript-es6-typeerror-class-constructor-client-cannot-be-invoked-without-ne

Dominique92 avatar Sep 17 '22 18:09 Dominique92

@Dominique92 perhaps try disabling transpiling by commenting out the buble plugin in https://github.com/jonataswalker/ol-geocoder/blob/master/build/config.js#L54

walkermatt avatar Sep 17 '22 20:09 walkermatt

I am working on the issue. The Base class in base.js calls super after accessing "this" which throws an error in ES6. Trying to refactor that.

EDIT: Done with this. ol-geocoder will now ship with ES6 classes. Will make a PR. Only issue I'm facing now is that the magnifying glass is not visible on the button since OL7 has default white background for colors. Will try to change color of magnifying glass.

Thanks for taking a look into it and fixing it :)

fogracvxy avatar Sep 17 '22 20:09 fogracvxy

@Dominique92 perhaps try disabling transpiling by commenting out the buble plugin in https://github.com/jonataswalker/ol-geocoder/blob/master/build/config.js#L54

Already did that. It works. Making up a PR after upgrading to v7 in examples

EDIT: PR is made. Please review it. #259

kirtan-desai avatar Sep 17 '22 21:09 kirtan-desai

@Dominique92 perhaps try disabling transpiling by commenting out the buble plugin in https://github.com/jonataswalker/ol-geocoder/blob/master/build/config.js#L54

Already did that. It works. Making up a PR after upgrading to v7 in examples

EDIT: PR is made. Please review it.

Can't download your repo using npm install getting errors with node-sass if installing with "no optional". If I do normal install it gets stuck at fsevents.

fogracvxy avatar Sep 18 '22 08:09 fogracvxy

I'm able to run npm i after cloning the repo.

kirtan-desai avatar Sep 18 '22 10:09 kirtan-desai

I'm able to run npm i after cloning the repo.

I am not sure what is happening with my npm install https://github.com/kirtan-desai/ol-geocoder.git stops with fsevents install and hangs there. npm install https://github.com/kirtan-desai/ol-geocoder.git --no-optional leaves me with node-sass errors Idk why...

fogracvxy avatar Sep 18 '22 11:09 fogracvxy

Anyway, it seems to me more logical to call super first, then to use this:

Doing this:

  ...
  this.layer = $nominatim.layer;
  super({ element: this.container });
}

If the parent constructor initialize this.layer, you will get his value.

is not the same than:

{
  super({ element: this.container });
  this.layer = $nominatim.layer;
  ...

you will get your value

Example : https://github.com/openlayers/openlayers/blob/main/src/ol/control/FullScreen.js#L81-L97

Dominique92 avatar Sep 18 '22 11:09 Dominique92

I'm able to run npm i after cloning the repo.

I am not sure what is happening with my npm install https://github.com/kirtan-desai/ol-geocoder.git stops with fsevents install and hangs there. npm install https://github.com/kirtan-desai/ol-geocoder.git --no-optional leaves me with node-sass errors Idk why...

installing with npm directly is also working for me. why don't you try cloning the repo and running it?

kirtan-desai avatar Sep 18 '22 11:09 kirtan-desai

@Dominique92 Did you take a look at my PR #259?

kirtan-desai avatar Sep 18 '22 11:09 kirtan-desai

@Dominique92 Did you take a look at my PR #259?

Sorry ! It's super :) Awaiting the dist (I don't use npm)

Dominique92 avatar Sep 18 '22 11:09 Dominique92

@Dominique92 Did you take a look at my PR #259?

Sorry ! It's super :) Awaiting the dist (I don't use npm)

Just clone my repo, run npm i and then run npm run build. You'll get the files in dist folder.

kirtan-desai avatar Sep 18 '22 12:09 kirtan-desai

Just clone my repo, run npm i and then run npm run build. You'll get the files in dist folder.

Sorry, I use Windows where npm is a mess, not linux I will wait the dist

Dominique92 avatar Sep 18 '22 13:09 Dominique92

Just clone my repo, run npm i and then run npm run build. You'll get the files in dist folder.

Sorry, I use Windows where npm is a mess, not linux I will wait the dist

I don't know when my PR will be reviewed. Till then, here are the dist files. Let me know if there's any issue.

kirtan-desai avatar Sep 19 '22 16:09 kirtan-desai

Till then, here are the dist files. Let me know if there's any issue.

Thank you. This dist works perfectly for me.

Dominique92 avatar Sep 19 '22 18:09 Dominique92

Is this repo maintained anymore? Should I publish a package of my forked repo as ol-geocoder v5?

kirtan-desai avatar Sep 21 '22 00:09 kirtan-desai

Hey guys, I have published my forked repo of ol-geocoder which is compatible with openlayers 7. I'll be maintaining the fork. https://www.npmjs.com/package/@kirtandesai/ol-geocoder

kirtan-desai avatar Oct 06 '22 15:10 kirtan-desai

Hi guys,

It's my first post... please be kind ;) Sorry to bother you but seems to me that the magnifying glass does not match very well with the new layout so, on my local copy, I've changed the base64 on the css with the following: 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAA8AAAAPCAYAAAA71pVKAAAAAXNSR0IArs4c6QAAAThJREFUOE+VkttxwkAMRXXN4NVf3AGkgtBBSAeUYDpIKog7CCU4FQQqgBJMB5SQ/Gn9WGXE2BnGNhnYrx1pz9XjLujiMPO8ruuZheq6PhLR92W+f0cbSOI4/gCQEtFPG3sgoo33/u2awBlm5oOqJiGEtKqqwmLT6XQxmUy2IYR9WZbrMQEwc6qqVmHeb9PGUNUCwEpEDoO2nXNbIjp571/H1C0PoBCRbABby0R0GEvaY+fchojm3vvVGJzbvGPJFra2t9cqL1V1D+ClP1ccxysAXwAeReQ0ahUzZ6r6rqpZCOG8mCiKlgAyAJ8iYhYOTucztVVsKU/tq52q5q3ApmmaorOxU/mD//lJCTMXqjoDsBaR/B64+0TPBl0K3FLZmMQ5Z7s4j9QJ3Ar3BY7e+8U9cCdgjuS2vF+pr50QDmmCjgAAAABJRU5ErkJggg=='

it is simply the 'original' with reverted colors. If you like it please feel free to use.

Massimo magnifying

MassimoGies avatar Oct 12 '22 15:10 MassimoGies

Hi guys,

It's my first post... please be kind ;) Sorry to bother you but seems to me that the magnifying glass does not match very well with the new layout so, on my local copy, I've changed the base64 on the css with the following: 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAA8AAAAPCAYAAAA71pVKAAAAAXNSR0IArs4c6QAAAThJREFUOE+VkttxwkAMRXXN4NVf3AGkgtBBSAeUYDpIKog7CCU4FQQqgBJMB5SQ/Gn9WGXE2BnGNhnYrx1pz9XjLujiMPO8ruuZheq6PhLR92W+f0cbSOI4/gCQEtFPG3sgoo33/u2awBlm5oOqJiGEtKqqwmLT6XQxmUy2IYR9WZbrMQEwc6qqVmHeb9PGUNUCwEpEDoO2nXNbIjp571/H1C0PoBCRbABby0R0GEvaY+fchojm3vvVGJzbvGPJFra2t9cqL1V1D+ClP1ccxysAXwAeReQ0ahUzZ6r6rqpZCOG8mCiKlgAyAJ8iYhYOTucztVVsKU/tq52q5q3ApmmaorOxU/mD//lJCTMXqjoDsBaR/B64+0TPBl0K3FLZmMQ5Z7s4j9QJ3Ar3BY7e+8U9cCdgjuS2vF+pr50QDmmCjgAAAABJRU5ErkJggg=='

it is simply the 'original' with reverted colors. If you like it please feel free to use.

Massimo magnifying

Hey Massimo, feel free to make this a PR on my fork!

kirtan-desai avatar Oct 12 '22 15:10 kirtan-desai