vasturiano / globe.gl

UI component for Globe Data Visualization using ThreeJS/WebGL
https://vasturiano.github.io/globe.gl/example/world-population/
MIT License
2.07k stars 306 forks source link

Link three.js as peer dependency #48

Open derwaldgeist opened 3 years ago

derwaldgeist commented 3 years ago

I am importing react-globe.gl as an ES6 modules on Meteor as a fullstack platform.

If I want to customize the globe, I have to import THREE as well, like this:

import THREE from 'three';

to be able to setup low-level stuff like a phong shader or lights.

But if I do, React complains about two simultaneously loaded three.js libraries:

image

This is because globe.gl references all three libraries as a hard dependencies and won't take the three I already provide. I've seen that it also checks for window.THREE, but this relies on a certain call order which you cannot guarantee in an ES6 environment.

If I omit three from my package.json, the import won't work anymore.

To resolve this, I'd like to suggest to use a peer dependency instead, so it is up to the developer to import three. The three-globe library already does it this way.

vasturiano commented 3 years ago

@derwaldgeist thanks for your suggestion.

However for specific case of this lib, three is really considered an internal implementation detail on how the webGL rendering is performed. For the majority of cases the user does not ever need to deal with ThreeJS details to be able to use this library, and thus it would be an unnecessary burden to ask the consumer app to always include this dependency explicitly. In this way, this is no different than any other regular lib dependencies of this lib, like tweenjs or kapsule. They're internal details that should remain as such. Where peerDependencies are most useful is when the lib itself is a component of a wider framework, like a React component for example. This is why three-globe has three as a peer dependency, because it is meant to be a "three plugin" and inserted into a ThreeJS scene, which is controlled by the consumer. Essentially, it is only a part of a whole. globe.gl is a different situation, it already includes all the ThreeJS scene/renderer scaffolding and it is available as a standard web component, and thus shouldn't require any peer dependencies.

Where three is slightly different than most libs is that it 1) creates a global variable and 2) shows a warning if more than one instance of it is detected in the whole dependency tree. None of these reasons are blockers however, it's simply a different architectural choice. And in addition, this component actually attempts to reconcile that by consuming from a global version of three if it is available at import time, which should be able to be guaranteed if the globe.gl import statements are placed after the three import.

derwaldgeist commented 3 years ago

Hi @vasturiano, thanks for sharing these insights. I understand your ratio for including three in globe.gl. The problem is that react-globe.gl is a wrapper around this, so it inherits the explicit dependency on three. And this actually causes problems, as mentioned above.

So far, I had no luck setting up react-globe.gl in a (modern ES6 modules) way that a) lets me customize it by modifying the scene, because this requires access to THREE classes, and b) avoiding two THREE instances at the same time. All the samples in react-globe.gl are using a simple HTML page to include the library, but this is not how it is typically done these days IMHO. And by "modifying the scene" I mean even simple things as modifying the shader.

Even if react-globe.gl gave me a way to access its own internal THREE, there would still be the problem that it won't interoperate nicely with other THREE-based code in the same app. Which I am planning to implement down the road. If react-globe.gl "forces" me to have two THREE implementations, that's kind of a road-blocker for me, especially as I am targeting mobile devices where app size counts.

Besides, I have tried to somehow implement a way to load THREE before react-globe.gl kicks in, by wrapping both in a React Loadable component (lazy loading). But still no luck. I've seen the code in globe.gl that tries to use the THREE instance provided by window.THREE, but it still did not work.

Here's my (simplified) code:

ThreeLoader.js:

import React from 'react';
import ReactLoadable from 'react-loadable';
import LoadingSpinner from './LoadingSpinner';

const ReactGlobeWrapper = ReactLoadable({
  loader: () => import('./ReactGlobeWrapper'),
  loading: LoadingSpinner
});

class ThreeLoader extends React.Component {
  constructor(props) {
    super(props);
    this.state = { loading: true };
  }
  componentDidMount() {
    import('three').then(three => {
      window.THREE = three;
      this.setState({ loading: false });
    });
  }
  render() {
    const { loading } = this.state;
    return loading ? null : <ReactGlobeWrapper {...this.props} />;
  }
}

export default ThreeLoader;

( I need the additional lazy loading of THREE because I am on mobile. The really interesting part is the lazy loading of the ReactGlobeWrapper using a Loadable component).

ReactGlobeWrapper.js:

import React from 'react';
import ReactGlobe from 'react-globe.gl';

const { useRef } = React;

console.log(window.THREE);

const ReactGlobeWrapper = props => {
  const globeEl = useRef();
  return <ReactGlobe ref={globeEl} />;
};

export default ReactGlobeWrapper;

The log confirms that window.THREE has been defined before the global code in ReactGlobeWrapper executes, and thus the deferred loading actually works. But still, ReactGlobe won't pick it up.

(Besides, this is still a pretty weird workaround.)

derwaldgeist commented 3 years ago

If you want to keep globe.gl's hard dependency on three intact (which I can fully understand), to me the only solution would be to decouple react-globe.gl from globe.gl and use peer dependencies to three, three-globe etc. there directly.

I understand why you have set it up this way (using Kapsule), but this three-staged wrapping unfortunately bears these issues.

Maybe you could make a forth library that is shared by react-globe.gl and globe.gl and which is the actual wrapper around three-globe for both of them, so globe.gl can still include everything (hard dependencies), but won't inherit it to react-globe.gl (which depends on that forth library directly)?

So instead of

react-globe.gl =>  globe.gl (hard) => three-globe (peer) => three (peer)

this:

react-globe.gl => three-globe-kapsule (peer) => three-globe (peer) => three (peer)
globe.gl => three-globe-kapsule (hard) => three-globe (peer) => three (peer)

Looks a bit more complex, but would solve the dependency problem and allow to share code as before, i.e. without duplicating the Kapsule encapsulation code. And it would allow us to use react-globe.gl in broader scenarios, where THREE is used for other purposes.

vasturiano commented 3 years ago

@derwaldgeist if your intent is not to have more than one version of Three in your app, all you should need to do is reconcile your app dependency tree so it's only importing one version (that would also help with reducing the bundle size). I'd recommend inspecting your dependencies lock file (package-lock.json or yarn.lock) and see if there is more than one version of three being included.

This module supports three >=0.118 so you should be able to find a (recent) version that satisfies both your app and this module. yarn-deduplicate is a handy tool for this.

In other words, it's not necessary to have dependency libs define their - potentially common dependencies with consuming apps - as peerDependencies to have a single version of a module installed in your app.

derwaldgeist commented 3 years ago

I have double-checked my package-lock.json and my node_modules folder. There is definitely only one version of three referenced: three@0.128.0.

Here's the output of npm list | grep three:

│ │ ├── three@0.128.0
│ │ ├─┬ three-globe@2.18.3
│ │ │ ├─┬ three-conic-polygon-geometry@1.4.3
│ │ │ ├── three-fatline@0.5.0
│ │ │ ├─┬ three-geojson-geometry@1.1.3
│ │ └─┬ three-render-objects@1.26.3
├── three@0.128.0

I think the problem arises because I have to import three directly in my React files, so I can access the THREE object for its classes. And then react-globe.gl(or better globe.gl) imports three again. This happens even if I ensure that window.THREE has already been set (double checked that).

snshn commented 3 years ago

Maybe there could be a way to expose the internally used THREE from three-globe/globe.gl? I, too, am using a separately imported instance of THREE to implement some parts of my scene, e.g. solar terminator. Being able to access the THREE that's already inside globe.gl/three-globe, would make my app more lightweight. Something like an utility method getThree() would do.

vasturiano commented 3 years ago

@snshn if you have only one version of three in your dependency tree, exposing the internal variable would have no effect on your bundle size. It would also be awkward to access the whole of three that way, as it should really be imported via ES modules to take advantage of tree-shaking.

snshn commented 3 years ago

@vasturiano ah, that makes sense. I've been prototyping my app using index.html, no build system yet.

ettoredn commented 1 year ago

Spent half a day dealing with why my customized globe (over globe.gl's custom layer) stopped working: globe.gl's directional light not added to the scene, the scene was brighter than before, etc.

Turns out yarn's was installing the latest version of threejs (which enabled color mapping by default) together with the exact version I specified in my project's package.json. As a result, globe.gl was using the latest version, while my project a previous, DIFFERENT, one.

Long story short, you have to use Yarn's resolutions to have only one version of threejs loaded.

I support the proposal of declaring three as a peer dependency so that projects with react-globe.gl and custom three objects do not break out of the blue after a while. Or, at least, add a visible warning somewhere in the README.

Nota bene: this behavior is present in Yarn v1, but not in pnpm. Haven't tested with Yarn v2.

vasturiano commented 1 year ago

@ettoredn this module will work with any version of ThreeJS >=0.118, as you can see from the package.json: https://github.com/vasturiano/globe.gl/blob/9256dc4b8e34c7900c3ed9963839b5423d7152af/package.json#L54

So you can always make it share the same version of Three that you have in your app (as long as that's recent enough), by doing some deduplication in your dependency tree. It doesn't force you to upgrade to the latest version of ThreeJS. Also moving it to peer dependencies wouldn't really be much different because the version range would be the same. And it doesn't make much sense to make it peer deps because this is not a ThreeJS plugin (not in the same sense that three-globe is for example).

About the scene being brighter that's due to a change in the latest version of ThreeJS (0.152) related to the color spaces. See more at: https://discourse.threejs.org/t/updates-to-color-management-in-three-js-r152/50791