pex-gl / pex-renderer

Physically based renderer (PBR) and scene graph for PEX.
https://pex-gl.github.io/pex-renderer/examples/index.html
MIT License
237 stars 16 forks source link

Component namespacing #314

Closed vorg closed 1 year ago

vorg commented 2 years ago

I miss destructuring a bit

const { components: { lights: { directional: directionalLight } } } = 
require("pex-renderer")

Any preference?

  1. (current)
    
    import {
    components,  
    } from "pex-renderer";

const e = { directionalLight: components.directionalLight() }


2.
```javascript
import {
  components,  
} from "pex-renderer";

const lights = components.lights

const e = {
  directionalLight: components.light.directional()
}

3.

import {
  directional as directionalLight,  
} from "pex-renderer/components/lights/directional.js";

const lights = components.lights

const e = {
  directionalLight: directionalLight()
}
dmnsgn commented 2 years ago

I would avoid 3. (deep import in packages currently cause me more headache and might confuse bundlers), keep pex-renderer exporting components and rely on tree-shaking.

Not sure it is necessary to sub-namespace components so I'd stick with 1.

vorg commented 2 years ago

@simonharrisco mentions option 4. - exposing all components on the top level, also not a bad idea

import { directionalLight } from "pex-renderer"

const e = {
  directionalLight: directionalLight()
}
dmnsgn commented 2 years ago
  1. could simplify export in index.js:
    export { default as directionalLight } from "./components/directional-light.js";
    // or
    export * from "./components/index.js";

    and also means only systems are namespaced?

vorg commented 2 years ago

and also means only systems are namespaced?

No, if we move components up then systems too.

dmnsgn commented 2 years ago

and also means only systems are namespaced?

No, if we move components up then systems too.

We'd run into conflict for animation (both component and system) unless systems are suffixed (animationSystem).

vorg commented 2 years ago

Well that was the idea for name-spacing in the first place which looks like brings us back to current option 1.

vorg commented 1 year ago

We decided to leave flat list of components. Maybe later if list gets too big.