Viglino / ol-ext

Cool extensions for Openlayers (ol) - animated clusters, CSS popup, Font Awesome symbol renderer, charts for statistical map (pie/bar), layer switcher, wikipedia layer, animations, canvas filters.
https://viglino.github.io/ol-ext/
Other
1.25k stars 463 forks source link

Openlayers 7 compatibility #839

Closed fracsi closed 1 year ago

fracsi commented 2 years ago

Current version seems to be incompatible with the new Openlayers 7.0.0.

Legend throws Class constructor BaseObject cannot be invoked without 'new' here: https://github.com/Viglino/ol-ext/blob/0cc449ad30a905207e5d9940c0c082f27261d5fb/src/legend/Legend.js#L39

Razi91 commented 2 years ago

Whole ol-ext would require rewritting now, OpenLayers changed building options, dropping legacy syntax. Now in bundle there are real classes that are much more restrict. I was using just AnimatedCluster, so I just locally wrote something like:

export class AnimatedCluster7 extends ol_layer_Vector {
  constructor(options) {
    // constructor
  }
  // other methods, previously injected through prototype

It's not hard job, but require a lot of work.

Anyway, it would be nice to rewrite it slowly in TypeScript, now that OL has own types in it's package.

Viglino commented 2 years ago

ol-ext has its own TypeScript def: https://github.com/Siedlerchr/types-ol-ext

Siedlerchr commented 2 years ago

@Razi91 VSCode as an option to automatically convert to class syntax. That's what I use as a base for creating the external typedefs.

The error comes because transpiled classes to ES5 cannot extend native ES6 ones https://stackoverflow.com/a/51860850 I am unfortunately not that deep into the gulp stuff to know if we can use it without

Here is an example how it looks like: https://github.com/Siedlerchr/ol-ext/tree/typejsdoc

mike-000 commented 2 years ago

The examples are now not working as the latest build and css are now at https://openlayers.org/en/latest/legacy/ol.css https://openlayers.org/en/latest/legacy/ol.js

That is not compatible with extending classes using ol_ext_inherits so the short term fix would be to change to https://openlayers.org/en/v6.15.1/css/ol.css https://openlayers.org/en/v6.15.1/build/ol.js

Viglino commented 2 years ago

The problem is to ensure compatibility with the pure js lib (https://viglino.github.io/ol-ext/dist/ol-ext.js). But ES6 class syntax is now widely supported in all modern browsers and could be used indeed (ending explorer support as ol7 does) I'll try to integrate it progressively...

Siedlerchr commented 2 years ago

I will update my fork asap, so you can use this PR then as a base.

Viglino commented 2 years ago

That is not compatible with extending classes using ol_ext_inherits so the short term fix would be to change to https://openlayers.org/en/v6.15.1/css/ol.css https://openlayers.org/en/v6.15.1/build/ol.js Examples are now working on v6.15.1 while updateing to ES6 classes

Viglino commented 2 years ago

@Razi91 VSCode as an option to automatically convert to class syntax. That's what I use as a base for creating the external typedefs.

I've just discovered this function 🤩 It's a kind of magic!

Viglino commented 2 years ago

@Siedlerchr tell me if you have something working. I'll may have some times to work on it tomorrow...

Viglino commented 2 years ago

I've started to move to ES6 classes. @Razi91 see https://viglino.github.io/ol-ext/examples/animation/map.animatedcluster.html @Siedlerchr I don't use your PR to add functionalities step by step (the whole bulk is to hard to debug), but your VS tip work like a charm... Tell me if you see something that can help to derive typescript defs.

Siedlerchr commented 2 years ago

@Viglino I basically just used VS Code's magic function. Nothing else. I tested a couple of examples, and it seems to work. I just also checked with peer deps update to ol 7. For typedefs, the javadoc is enough if the types (e.g. number or the ol ones can be correctly inferred.

vitalus commented 2 years ago

Yes. Classes are needed. I solved problem with OL 7 and ol-ext using webpack and babel-loader: to force to transpile /node_modules/ol/* into JS without classes. As a workaround. It works.

image

So, plugin "@babel/plugin-transform-classes" converts classes into functions.. and legacy ol-ext continues working (until classes are created, I hope).

Siedlerchr commented 2 years ago

@Viglino I recently tried to create an example in the types for the video recorder function and noticed that I already have problems with the GeoPortail Layer that cannot be cast to the BaseLayer class. Would be nice if you could take a look at that hierarchy while transforming this to the classes

Viglino commented 2 years ago

@Siedlerchr I'm slowly moving to class declaration (actually I'm in holidays). Geoportail Layer are now using class declaration...

Siedlerchr commented 2 years ago

@Viglino Thanks for the continuous work. Enjoy your holidays :) Edit and thanks for the clarification of the class expression thing.

Viglino commented 2 years ago

I'm almost done upgrading to ES6 classes. All examples are using ol v.7 I still have to check some extra classes and lint... Try to publish a new version asap.

Viglino commented 2 years ago

@mike-000 I've noticed a bug on VectorLayer opacity. It seems it is applied twice. You can see it on: https://viglino.github.io/ol-ext/examples/layer/test.html The first time the opacity is set is right. But when you move the map the layer fade.

Viglino commented 2 years ago

I've published a new version 4.0.0 👉 4️⃣.0️⃣.0️⃣ https://www.npmjs.com/package/ol-ext Now compatible with ol v7 Tell me if you see bugs 🐞

Siedlerchr commented 2 years ago

I will try to work on updated types.

Viglino commented 2 years ago

😃 Official release : https://twitter.com/jmviglino/status/1573291101791948800 image

Siedlerchr commented 2 years ago

Oh need to make a new release for the types then

Viglino commented 2 years ago

Oh need to make a new release for the types then

no pressure...

Siedlerchr commented 2 years ago

@Viglino Just released a new version 3.0.0 of the types package. Now compatible with ol-ext 4.01 and ol7

leegee commented 1 year ago

Does ol-ext now have OL7 compatibility, ie should this issue remain open?

Would it be worth mentioning compatibility status in the README.md?

Viglino commented 1 year ago

ol-ext is compatible with ol7

mike-000 commented 1 year ago

I think the minified build in #901 was not compatible but in the latest version that is also fixed?

Note that all the examples are still using OpenLayers 7.0.0 as the legacy path was not used after that, the latest released versions are now at

https://openlayers.org/en/latest/ol/dist/ol.js https://openlayers.org/en/latest/ol/ol.css