Volumetrics-io / mrjs

An extensible WebComponents library for the Spatial Web
https://mrjs.io
MIT License
148 stars 9 forks source link

noodle - inheritance not being handled right - needs documentation update for expln #619

Closed hanbollar closed 1 month ago

hanbollar commented 1 month ago

Linking

Fixes #618

Problem

Screenshot 2024-05-07 at 3 29 00 PM

(see the rest in the issue description)

something is wrong with inheritance for user designated items

Solution

Quick explanation of change to be done

needs more noodling first

Breaking Change

If this is a breaking change describe the before and after and why the change was necessary

Notes

Notes and any associated research or links


Required to Merge

render[bot] commented 1 month ago

Your Render PR Server URL is https://examples-mrjs-pr-619.onrender.com.

Follow its progress at https://dashboard.render.com/static/srv-cotamkmv3ddc73crbk00.

hanbollar commented 1 month ago

The issue

It has to do with mr.js taking longer to load - so even though mr.js /isnt/ an async load and you /should/ be able to do :

<script src="./mr.js">
<script>
  class MRFoo extends MREntity {
     ...
  }
</script>

which gives us the sad state where the model doesnt load under MRFoo

Screenshot 2024-05-10 at 7 15 38 PM

which in our case means, MREntity finally becomes defined after MRFoo is already being handled, which is /not/ what we want.

Screenshot 2024-05-10 at 7 15 49 PM

The reasoning and short term fix

mrjs is big enough that it takes some time for its items to load one by one - which means unfortunately we end up with a need to have people add items to extend through async if they want mrjs to be handled 100% as expected

that is, at the moment we need them to do something like:

<!-- MRjs -->
<script>
function defineMRFoo() {
  class MRFoo extends MREntity {
     ...        
  }
</script>
<script src="./mr.js" onload="defineMRFoo()"></script>

which means we get what we wanted:

Screenshot 2024-05-10 at 7 03 36 PM

MRFoo

note: we currently define MRFoo as follows:

class MRFoo extends MREntity {
    constructor() {
        super();
    }

    connected() {
        super.connected();
        this.boxMesh = new THREE.Mesh(
            new THREE.BoxGeometry(0.2, 0.2, 0.2),
            new THREE.MeshPhongMaterial({
                color: "#ff9900",
                transparent: true,
                opacity: 0.4,
            }));
        this.object3D.add(this.boxMesh);
    }
}

// Define custom element
customElements.get('mr-foo') || customElements.define('mr-foo', MRFoo);
hanbollar commented 1 month ago

@lobau @michaelthatsit let me know if you have any experience with delayed loading of libraries for handling in the html directly - i dont really want to force users to do an onload function to extend anything from our mr.js lib

hanbollar commented 1 month ago

talked with @michaelthatsit and @lobau

some things:

hanbollar commented 1 month ago
  • try async connected and adding await in front of super.connected() ??

doesnt work - hitting same issue of :

Screenshot 2024-05-13 at 11 20 40 AM
hanbollar commented 1 month ago

double defer doesnt work - takes too long too load

Screenshot 2024-05-13 at 11 49 34 AM
hanbollar commented 1 month ago
Screenshot 2024-05-13 at 12 53 49 PM

It seems to work ok if we have src for mrjs script in header and the class definition in the header

this is because we are using mrfoo as an html element within the group and then defining it after - so all the asyncs we're doing for the scripts wont work because of the html tag predefinition. The onload function version works from https://github.com/Volumetrics-io/mrjs/pull/619#issuecomment-2105457895 since it blocks everything related until after.

What i find is interesting is that adding defer to the source script in header of mrjs and the creation script of class MRFoo which is after the body, doesnt have the same effect as the onload function does. But from what I understand, that's because the onload function links the two together so MRFoo still ends up being created fully before the body's html information gets executed.

hanbollar commented 1 month ago

Solved!

Starting Notes

@lobau @michaelthatsit - it seems to work all fine if we have users define things in the header after the Githubissues.

  • Githubissues is a development platform for aggregating issues.