Skip to content

fix: added missing js reference for map layers #8

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

CatchABus
Copy link
Contributor

Right now, there are a couple of cases of weak references being lost because carto map does not keep refs of layers on JS-side.

This PR ensures that we keep a proper reference of Layers instance and its layers using a JS array.
It also corrects a few types and keeps all common methods related to Layers in common file.

Additionally, method getLayers becomes lazy and instantiates Layers once during first execution, provided that native view is already initialized.
Getter mapView becomes common and is now added to types.
Certain Layers methods returned wrong type. See https://cartodb.github.io/mobile-android-samples/reference/com/carto/components/Layers.html#removeAll(com.carto.layers.LayerVector)
Methods addAll, setAll, and removeAll have been fixed as they called certain other Layers instance methods without this context.
Example of the problem:

removeAll(layers: Layer<any, any>[]) {
    layers.forEach(this.remove); // Method remove will be executed as a plain callback, missing context
}

@farfromrefug
Copy link
Member

@CatchABus complex to read from my phone. But it looks ok to me. You tested about all.methods ?

@CatchABus
Copy link
Contributor Author

CatchABus commented May 11, 2025

@CatchABus complex to read from my phone. But it looks ok to me. You tested about all.methods ?

I have injected the changes in node_modules and am currently using them in our app!

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

Successfully merging this pull request may close these issues.

2 participants