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

TypeError: Cannot read properties of undefined (reading 'sky') #4340

Closed
oterral opened this issue Jun 28, 2024 · 17 comments · Fixed by #4431
Closed

TypeError: Cannot read properties of undefined (reading 'sky') #4340

oterral opened this issue Jun 28, 2024 · 17 comments · Fixed by #4431
Labels
need more info Further information is requested

Comments

@oterral
Copy link
Contributor

oterral commented Jun 28, 2024

The error comes from the line https://github.com/maplibre/maplibre-gl-js/blob/main/src/render/painter.ts#L412

The stylesheet object doesn't exist when we call map.redraw() before the style is loaded. I guess the line should be

if (this.style.sky) drawSky(this, this.style.sky);

maplibre-gl-js version: 4.5.0

browser: all

Steps to Trigger Behavior

  1. Create a map
  2. Call map.redraw()

Link to Demonstration

https://codepen.io/oterral/pen/PovvbJx

Expected Behavior

No error about sky property

Actual Behavior

A TypeError is triggered in the console

@HarelM
Copy link
Collaborator

HarelM commented Jun 28, 2024

Why calling redraw before the style is loaded?

@HarelM HarelM added the need more info Further information is requested label Jun 28, 2024
@oterral
Copy link
Contributor Author

oterral commented Jun 28, 2024

We do this in https://github.com/geoblocks/ol-maplibre-layer . More precisely we call redraw on each render frame.

@oterral
Copy link
Contributor Author

oterral commented Jun 28, 2024

What is weird, is this.style exists so why testing this.style.stylesheet.sky and don't reuse it in renderSky ?

@HarelM
Copy link
Collaborator

HarelM commented Jun 28, 2024

I wanted sky to alway be initialized to avoid these if statements. stylesheet.sky is the sky specification and not the sky object...

@HarelM
Copy link
Collaborator

HarelM commented Jun 28, 2024

See here:
#3645 (comment)

@oterral
Copy link
Contributor Author

oterral commented Jul 2, 2024

ah ok so may be just a beffer "if" condition will get rid of the error without changing anything to the logic:

if (this.style.stylesheet?.sky) drawSky(this, this.style.sky);

@HarelM
Copy link
Collaborator

HarelM commented Jul 2, 2024

That might work 😃 I have no real objections.
Would be interesting to understand how other parts are working so that they don't fail like that...

@am2222
Copy link

am2222 commented Jul 6, 2024

I faced similar error using maplibre with react-map-gl with https://api.maptiler.com/maps/streets/style.json mapStyle. I think the older styles might be missing sky properties?

@HarelM
Copy link
Collaborator

HarelM commented Jul 6, 2024

We have tested old styles that obviously don't have sky properties, and they work.
It seems to be related to how this library is being used, which we didn't take into consideration like the usage of redraw before map load event, and possibly other scenarios.

@am2222
Copy link

am2222 commented Jul 6, 2024

@HarelM thanks for the response! So I noticed downgrading its version to 4.5.1 solves the issue.

@HarelM
Copy link
Collaborator

HarelM commented Jul 7, 2024

This feature was introduced as part of 4.5.0 version, downgrading to previous versions will probably solve this.

@UberMouse
Copy link

We hit the same issue in our application. Where we manage custom 3d layers synchronisation, after creating/deleting the custom layers we call map.redraw() to paint it (though tbh I don't think we actually need to do that) and that causes the issue. We run a fork of maplibre to address #3983 so I applied the null safety operator to the if check outlined above and the issue did stop happening.

@birdofpreyru
Copy link
Contributor

I've also bumped into this issue just now. I had no time to carefully investigate it yet, but it feels like it happens reliably when the map component is removed from DOM. I am also using this library indirectly, through https://github.com/visgl/react-map-gl — it as well might be a bug there.

@birdofpreyru
Copy link
Contributor

@HarelM

  1. I checked that adding ? into if (this.style.stylesheet?.sky) drawSky(this, this.style.sky);, suggested by @oterral fixes the problem for me.
  2. I think, what happens is the following (I am not familiar with Maplibre codebase, I just had a quick look at it) — when the map component is unmounted all layers are removed first, while the Map object may still exist. At that point the style object is replaced by an empty Style, which has no stylesheet field attached (it seems to be attached only after some data layers are loaded); at that point the map is already marked as dirty, and might be re-rendered, which fails at that point because it assumes that stylesheet is always attached when the map is rendered.

@birdofpreyru
Copy link
Contributor

Can we get this patched ASAP? Do you want me to create a PR?

@HarelM
Copy link
Collaborator

HarelM commented Jul 19, 2024

Please open a PR, yes.

@birdofpreyru
Copy link
Contributor

Here you are: #4431

HarelM pushed a commit that referenced this issue Jul 22, 2024
* [#4340] Fixes possible access to undefined object

* CHANGELOG update

* Adds the test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need more info Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants