openlayers / ol-cesium

OpenLayers - Cesium integration
http://openlayers.org/ol-cesium/
BSD 2-Clause "Simplified" License
1.01k stars 327 forks source link

Why doesn't ol-cesium import Cesium? #953

Closed ruckc closed 1 year ago

ruckc commented 3 years ago

I'm trying to use ol-cesium inside a react app, built with create-react-app. When using ol-cesium, i get the Cesium is not defined error. I see the references to loading Cesium onto the window object, and i've tried importing Cesium in the root index.html, with no luck.

I'm trying to load it with:

<script type="text/javascript" src="node_modules/cesium/Build/Cesium/Cesium.js"></script>

I see the network tab load the .js, but its not globally available on the window object. So why doesn't ol-cesium just import Cesium fromcesium/Cesium'`. It seems like that would be a fairly standard way of loading the library (in addition to having a cesium dependency defined in package.json).

b4l8ter commented 3 years ago

I'm experiencing the exact same Cesium import issue building a vue.js app. Like you I have reverted to an 'old-fashioned' include via script tag and src attribute. Unlike you, this method works fine for me. Your problem is likely the relative path for src is NOT accessible at run time. I put my Cesium.js in a subfolder of public and referenced as <script src="./libs/Cesium/Cesium.js"></script>

gberaudo commented 3 years ago

It is mainly for historical reasons: CesiumJS was not available as ES6 modules at the start of the ol-cesium project. In addition:

That being said it may be time to drop the "old-fashioned" way of using ol-cesium and as a consequence the loading of CesiumJS this way. So I will keep this issue open.

b4l8ter commented 3 years ago
  • it works the same for people using ol-cesium the ES6 way or the "old-fashioned" way;

We can't get it to work the ES6 way.

ruckc commented 3 years ago

@gberaudo - could we do conditional importing? if its not already defined import it and make the dependency optional? or just rev the major version to drop backwards compatibility?

gberaudo commented 3 years ago

@b4l8ter, how did you do? Did you check how the examples works?

gberaudo commented 3 years ago

@ruckc, ES6 does not allow conditionnal imports, unless you use the asynchronous import() function, but using that would be certainly painful to work with and result in bundlers generating non-optimal code. My idea is more to drop completly the support for 'old-fashioned' builds, in a non-backward compatible update. But this all depends whether this feature is used or not and also the impact on current code: will it still be easy to lazy load the CesiumJS part on demand, like it is currently done in https://github.com/openlayers/ol-cesium/blob/master/examples/lazy.html

b4l8ter commented 3 years ago

@b4l8ter, how did you do? Did you check how the examples works?

  • ol-cesium examples are using ES6 imports of ol-cesium (with separate "old-fashioned" Cesium);
  • ol-cesium example oldfashioned.html is using both ol-cesium and cesium as external "old-fashioned" libraries.

Sorry but I'm not sure I understand the difference in the two methods you mention. I'm using ES6 import of ol-cesium and (old fashion) direct load of Cesium.js using script/src in my html. This is the only way I can make it work at the moment. As far as I can tell, all the examples seem to use this same old-fashion method. The inject_ol_cesium.js is basically doing the same thing (referencing either dev or minified Cesium build). Also confused why this file is not named "'inject_cesium.js"' as it injects Cesium not ol-cesium?

gberaudo commented 3 years ago

@b4l8ter, Your use of ol-cesium is the standard one. Some people use ol-cesium, ol and CesiumJS as "old-fashioned" libs (no ES6 modules at all).

This is historical, "inject_ol_cesium.js" used to load OL, hence its name.

mholthausen commented 3 years ago

Unfortunately, I can't get ol-cesium to run with the ol-app either. I have made the following:

mkdir olcesium_test
cd olcesium_test
npx create-ol-app
npm i
npm start

--> ol map is running fine, npm stopped. npm i --save olcs Then extend the main.js file as follows:

import { Map, View } from 'ol';
import TileLayer from 'ol/layer/Tile';
import OSM from 'ol/source/OSM';
import OLCesium from 'olcs/OLCesium.js';

const map = new Map({
  target: 'map',
  layers: [
    new TileLayer({
      source: new OSM()
    })
  ],
  view: new View({
    center: [0, 0],
    zoom: 2
  })
});

const ol3d = new OLCesium({ map }); // ol2dMap is the ol.Map instance
ol3d.setEnabled(true);

npm start The 2D map can be displayed, but the following message from OLCesium.js (from the NPM package itself) appears:

OLCesium.js:170 Uncaught ReferenceError: Cesium is not defined
    at new OLCesium (OLCesium.js:170)
    at Object.hTXjU.ol (main.js:19)
    at newRequire (index.6b9ca3e8.js:71)
    at index.6b9ca3e8.js:120
    at index.6b9ca3e8.js:143

Are there any further adjustments to be made there, so that ol-cesium is executable within the create-ol-app?

I am using ol-cesium in version 2.13 and have also run additional npm i cesium@1.82, but this also does not change the error message.

b4l8ter commented 3 years ago

@mholthausen I don't think there is anything we can do until ol-cesium is updated to support CesiumJS ES6 modules. Hopefully it will be updated soon (CesiumJS migrated to ES6 about 2 years ago).

ruckc commented 3 years ago

What need to happen to update ol-cesium to use ES6?

gberaudo commented 3 years ago

@mholthausen, you should add the Cesium external dependency. See #979.

MM12300 commented 2 years ago

First I would like to thanks all the contributors to this project, its obviously an insane workload. Second, I would like to agree on keeping this issue open because it's quite surprising to have this error while only copying 3 lines of the get-started.

Why don't you add a reference to this issue or just a line like "cesium is not linked to cesium-ol so pleaase link the cesium lib locally or with a cdn in order to have this example work" ? I don't remember if the npm page can be edited.

gberaudo commented 2 years ago

Hi @MM12300, I added some notes about it in the README.

MM12300 commented 2 years ago

Thanks ! :)