Skip to content

Wrong type on OlSourceVector.features #399

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
azygis opened this issue Dec 17, 2024 · 4 comments · Fixed by #400
Closed

Wrong type on OlSourceVector.features #399

azygis opened this issue Dec 17, 2024 · 4 comments · Fixed by #400
Labels
bug Something isn't working

Comments

@azygis
Copy link
Contributor

azygis commented Dec 17, 2024

Describe the bug
The type on features prop of OlSourceVector is wrong (or incomplete). Currently it accepts array / collection of Geometry but it should be Feature or even FeatureLike. This forces us to add nasty hacks like const realFeatures = computed(() => features.value as unknown as Geometry[]); and passing that instead.

Affected version(s)

Pretty sure all version up until now.

To Reproduce
N/A

Expected behavior
Above.

Screenshots
N/A

Desktop (please complete the following information):
N/A

Smartphone (please complete the following information):
N/A

Additional context
I can create a pull request with the change but I'm not sure if it will become a breaking change or not. Would like to know if it was an oversight or an intentional implementation.

@azygis azygis added the bug Something isn't working label Dec 17, 2024
@d-koppenhagen
Copy link
Collaborator

Hey, please send me a PR. I will check it and think about if a breaking change is necessary.

@azygis azygis mentioned this issue Dec 18, 2024
10 tasks
@azygis
Copy link
Contributor Author

azygis commented Jan 25, 2025

I'm in process of fixing all the types. There's a lot of mix-n-match, sometimes separate types which are not exactly like in ol (like for some of conditions), some are straight wrong. Changing everything to "proxy" the types from ol options. Since I'm doing that on my off time, it's not going as fast. Depends on when the PR gets merged, either pushing to that or a new PR.

@azygis
Copy link
Contributor Author

azygis commented Feb 11, 2025

All typings have been corrected, using the exported Options types as close as vue compiler can handle. Intellisense works better as well now, including majority of events.

@d-koppenhagen @MelihAltintas can someone review? I'd like to have this merged first before starting to tackle #378.

d-koppenhagen pushed a commit that referenced this issue Feb 11, 2025
* Fix some of TS typing issues

* Fix the majority of other TS issues

* Added missing default for ol-options injection

* Found some more lingering issues

closes #399

---------

Co-authored-by: Žygimantas Arūna <zygimantas.aruna@roxit.nl>
@d-koppenhagen
Copy link
Collaborator

@azygis great job!, thanks for your contribution! I will cut a new version soon

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants