Skip to content

fix: prevent app wide crash when terrain properties are undefined #2582

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.

Sign up for GitHub

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
mbret wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: prevent app wide crash when terrain properties are undefined #2582

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion modules/react-mapbox/src/components/source.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,19 @@ export function Source(props: SourceProps) {
}
}
}
map.removeSource(id);

/**
* There is a bug in mapbox where style.terrain.properties may be undefined.
* @see https://github.com/mapbox/mapbox-gl-js/blob/7a72385de5c7400647ea7d3539637145fdf616a7/src/terrain/terrain.ts#L380
* To prevent this component to crash, we catch it for now.
* This seems to be happening the majority of time with <StrictMode> but not only.
*/
try {
map.removeSource(id);
} catch (error) {
// eslint-disable-next-line
console.error(error);
Comment on lines +120 to +122
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Re-throw unexpected removeSource failures

This catch block swallows all map.removeSource exceptions and only logs them, which leaves the source mounted when cleanup fails; that can silently corrupt later renders because map.getSource(id) will return stale data on remount (especially with explicit ids), and in StrictMode/unmount-heavy flows it can accumulate orphaned auto-generated sources instead of actually cleaning up. Limiting the catch to the known terrain bug condition (and rethrowing otherwise) would avoid masking real cleanup failures.

Useful? React with 👍 / 👎.

Copy link
Author

@mbret mbret Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codex, source is correctlyl removed, only the following updating terrain path is failing. Which in this situation is unrecoverable anyway.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To use Codex here, create an environment for this repo.

}
}
};
}
Expand Down