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

Update to OpenLayers 5.0 #181

Open
mhosman opened this issue Jun 28, 2018 · 24 comments
Open

Update to OpenLayers 5.0 #181

mhosman opened this issue Jun 28, 2018 · 24 comments

Comments

@mhosman
Copy link

mhosman commented Jun 28, 2018

New OpenLayers 5.0 has been released. It could be great to update this library to support that version.
https://github.com/openlayers/openlayers/releases

@davinkevin
Copy link
Contributor

We will first move to angular 6 packaging and then migrate openlayers. But yes, this should be done

@OSHistory
Copy link

There are plans to include typings within openlayers itself (in progress). Also note that now only the ol-package is supported.

openlayers/openlayers#8120

@samuel-girard
Copy link

Hi @mhosman
I started some work to include OpenLayers 5 in ngx-openlayers here: #183
There is nothing to show yet, but very soon I hope.

@mhosman
Copy link
Author

mhosman commented Sep 6, 2018

Any new about this? Thank you!

@samuel-girard
Copy link

Hello @mhosman
The migration to OL5 is still going on.
We are just facing one last issue: the lack of proper typings. As there is some work being done on OpenLayers side to add type checking to the library, we are waiting for a release that will include this.
I'll update this thread when we have something that can be used.

@OSHistory
Copy link

It seems like typings inside the ol-package probably won't be provided for some time :(. I started
an effort to generate them from source in a purely automated fashion. The main-script is called
hot-fix-content.py, and indeed the project is quite frankly a bit messy, but the resulting types do
at least a decent job (biggest caveat is missing type-declarations in functions and return types).

Perhaps you might be interested in testing them out for your migration to OL5. I don't want to release
them to DefinetlyTyped until there has been some testing by other developers and more
typing has been added (see above).

https://github.com/OSHistory/generate_ol_types

https://github.com/OSHistory/generate_ol_types/releases/tag/v1.0.0-alpha.1

@HarelM
Copy link
Contributor

HarelM commented Feb 3, 2019

Any news about this? Does the next branch has this?
I would like to use ol-mapbox-style with this library and it seems that it requires OL 5...

@cyberrranger
Copy link

wait too

@HarelM
Copy link
Contributor

HarelM commented Feb 21, 2019

@maintainers can someone comment on this, please...

@davinkevin
Copy link
Contributor

To help for this one, some of you can solve conflicts on the PR #183 ?

@HarelM
Copy link
Contributor

HarelM commented Feb 21, 2019

I'd be happy to, but I don't have write access to either of the repositories... Is there a way for me to help solving the conflict using some git magic?

@davinkevin
Copy link
Contributor

You can fork this code in another PR if you want and solve this.

@HarelM
Copy link
Contributor

HarelM commented Feb 21, 2019

Just to make sure I follow your suggestion:

  1. Fork the repository used by @samuel-girard
  2. Add the relevant code there to solve the conflicts (I'm not sure I know how to do it since I'll need to get the latest changes from this repository which won't be the parent of my repository)
  3. Create another PR that will be used instead of OpenLayers 5 #183
    Did i get it right?

I don't mind doing so, but the same problem can occur again - I'm guessing the original PR did not have conflicts but they started to appear with time since it was not merged.
Are you guys ready to merge a pull request with the relevant changes?

@davinkevin
Copy link
Contributor

davinkevin commented Feb 21, 2019 via email

@samuel-girard
Copy link

samuel-girard commented Feb 22, 2019

Hello @HarelM

I don't mind doing so, but the same problem can occur again - I'm guessing the original PR did not have conflicts but they started to appear with time since it was not merged.

You are right, the same problem will occur again if another MR is merged into next before #183
I suggest to do the rebase on next only when the team (ping @Neonox31) is ready to merge this one, as it is a bit annoying without OpenLayers typings (I have done that many times).
Once they are ready, I can help you with that or do it myself when I have some spare time.

It would be nice to see this before OpenLayers 6 comes out ;-)

@HarelM
Copy link
Contributor

HarelM commented Mar 2, 2019

@Neonox31 can you share your status regarding this issue?

@HarelM
Copy link
Contributor

HarelM commented Mar 8, 2019

@samuel-girard do you think we should create a npm package from your repository until someone here answers? I really need ol 5 for ol-mapbox-style conversion...

@samuel-girard
Copy link

@HarelM, yes we can create a temporary package with ol5 that you can use until the branch is merged in ngx-openlayers.
Not sure if I will have enough time for that, but I'll give it a look this week end.

@samuel-girard
Copy link

samuel-girard commented Mar 10, 2019

@HarelM I just published ngx-ol on npm. I quickly tested on the demo project of ngx-openlayers and everything seems good.
To use it, you just need to do these things in your project:

  • in your package.json, replace ngx-openlayers by ngx-ol (version 0.0.1), then run npm install
  • add a dependency to ol (version 5.3.1 is good) - and remove dependency to openlayers and @types/openlayers if any
  • in your code replace all references to ngx-openlayers by ngx-ol (import { Stuff } from 'ngx-openlayers'; => import { Stuff } from 'ngx-ol';)

I'll remove this package once the ol5 branch is merged and published in ngx-openlayers.

@HarelM
Copy link
Contributor

HarelM commented Mar 10, 2019

@samuel-girard Thanks!!! Did you happen to take by any chance the latest changes from next version?
I added a boolean to vector layer which I very much need here:
#198
Very simple commit that can be seen here:
https://github.com/IsraelHikingMap/ngx-openlayers/commit/39bfbf4396adaf4d1f5cb5575faefeea6b660c88

I can create a PR to your repo if need be...

@samuel-girard
Copy link

Yes, I also rebased the branch on next before creating the package, so your change should be there.

@HarelM
Copy link
Contributor

HarelM commented Mar 11, 2019

@samuel-girard you're a life saver!! :-) I've used your package to migrate my site to ol 5 and everything seems to be working as expected! Yay!
BTW, which branch did you publish to NPM? seems like airbusgeo:openlayers_five does not have my changes so I'm guessing it is not the one you used, right?

@HarelM
Copy link
Contributor

HarelM commented Mar 11, 2019

@OSHistory I'll be happy to try out your typing, please let me know how to do it - your repository does not have the typings inside so I won't be able to use npm install git+... to get them... I've seen that you created a project that can have the typings using JSDocs, but I as far as I understand it only works for vscode, and I'm using visual studio...

@samuel-girard
Copy link

@HarelM yes, your changes are in the branch airbusgeo:openlayers_five (as seen here https://github.com/airbusgeo/ngx-openlayers/blob/a060d5d75bb35226f6d6a8846a2215289ab7d1fa/projects/ngx-openlayers/src/lib/layers/layervector.component.ts#L25).
airbusgeo:openlayers_five is the branch I published on NPM, so your changes are also in the published package.

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