-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Maps] Replace mapbox-gl import with maplibre-gl #104505
Conversation
Revealed #104518 |
drop in replacement seems to fail. compilation tripping over the https://github.com/elastic/kibana/pull/104505/checks?check_run_id=3001756317
|
TS-compilation failures seems to have been introduced by maplibre/maplibre-gl-js#30 , which seems to be a 1.15.0 feature. The new addProtocol/removeProtocol functions are missing return types, which is tripping up Kibana's type-checker. |
Upgrading to 1.15.2, which includes the TS-type fix 367a863 |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
with #109082, need to resolve merge conflicts |
Pinging @elastic/kibana-gis (Team:Geo) |
@elasticmachine merge upstream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thomasneirynck can you try the suggestions I left above?
Co-authored-by: Tiago Costa <[email protected]>
Co-authored-by: Tiago Costa <[email protected]>
@thomasneirynck for the version of types we were declaring those were deprecated (https://www.npmjs.com/package/@types/maplibre-gl) and also made available on the package itself, so I think we were having a conflict there, thats why I've suggested to stop use the types package and only define a dependency on the npm package which already bundles the types |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@elasticmachine merge upstream |
* replace import * fix import * upgrade * Update packages/kbn-mapbox-gl/BUILD.bazel Co-authored-by: Tiago Costa <[email protected]> * Update package.json Co-authored-by: Tiago Costa <[email protected]> * commit yarn lock * yarn lock Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Tiago Costa <[email protected]>
* replace import * fix import * upgrade * Update packages/kbn-mapbox-gl/BUILD.bazel Co-authored-by: Tiago Costa <[email protected]> * Update package.json Co-authored-by: Tiago Costa <[email protected]> * commit yarn lock * yarn lock Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Tiago Costa <[email protected]> Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Tiago Costa <[email protected]>
💔 Build Failed
Failed CI StepsTest FailuresKibana Pipeline / general / X-Pack Reporting API Integration Tests.x-pack/test/reporting_api_integration/reporting_and_security/ilm_migration_apis·ts.Reporting APIs ILM policy migration APIs detects when no migration is neededStandard Out
Stack Trace
Kibana Pipeline / general / X-Pack Reporting API Integration Tests.x-pack/test/reporting_api_integration/reporting_and_security/ilm_migration_apis·ts.Reporting APIs ILM policy migration APIs "after each" hook for "detects when no migration is needed"Standard Out
Stack Trace
Kibana Pipeline / general / X-Pack API Integration Tests.x-pack/test/api_integration/apis/uptime/rest/telemetry_collectors_fleet·ts.apis uptime uptime REST endpoints with generated data telemetry collectors fleet should receive expected results for fleet managed monitors after calling monitor loggingStandard Out
Stack Trace
and 1 more failures, only showing the first 3. Metrics [docs]Async chunks
Unknown metric groupsmiscellaneous assets size
History
To update your PR or re-run it, just comment with: |
Closes #108742.
Replaces mapbox-gl v1.13.1 with maplibre-gl v1.15.2.