-
Notifications
You must be signed in to change notification settings - Fork 63
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
createDom produces intermediate DOM states #110
Comments
After talking w/ some colleagues it seems like moving We still need a solution for Custom Elements, observers, and proxies that depend on a complete DOM node being added so it can analyze and maintain state. Maybe we can add an event like |
Hello, Thanks for trying maquette to do cool new things with! The whole idea of attaching nodes to the DOM before initializing them is to give external libraries/widgets like google-maps, recaptcha and ckeditor an attached DOM node, otherwise they will not work. It seems that A-Frame uses a kind of observer mechanism to determine when to start parsing the DOM. Maquette provides the afterCreate callback that would be the right moment to start. Do you have a repository or an online example that I can look at and debug? |
Sure, I have a repo up here: https://github.com/devpaul/derpymon It would be nice if we had control over when A-Frame parses the DOM and could tell it to wait for For maquette, I was thinking that adding an event that is triggered before a node is inserted or attached might provide a solution.
In the above event, if it exists and if it returns false then the node wouldn't be attached to the DOM. This would allow anyone to cache the node and return false and wait for |
It looks like maquette causes the One approach would be to test every Element for an 'attachedCallback' property (or check for dashes in the tagname) and if they have one, insert them into the DOM tree after initializing properties and children. But this will influence performance for people not using custom elements. |
I was thinking about something along those lines. On my machine I tested this by moving Maybe something like this would work as more formal solution? This would allow us to handle the corner case where a complete DOM is necessary while creating the smallest impact on normal virtual-DOM approaches. In cases where we want to delay attaching a node to the DOM until after it's properties and children are initialized we could do this by creating a Let me know what you think of this solution. If it's viable, I'll build it out and test it against my A-Frame project, add tests and submit a PR. |
Before making changes, lets have a look at the big picture here. Maquette provides the afterCreate (and afterUpdate) callback for components/widgets that are not based on custom elements. AfterCreate guarantees that the Element's properties and children have been initialized and the DOM node is (as far as maquette can tell) connected to the DOM, so the component/widget can determine the size and position of the element. (This does cause an extra reflow, thus hurting performance, but it works). Now lets enter the custom-components era (whenever that may be). Components based on the customComponents spec have a nice So for custom elements in general it would be better (and for A-frame even required) if maquette would add elements to the document as late as possible. I am assuming that these custom elements will not use afterCreate to initialize pre-custom-component widgets which rely on the DOM being attached/connected, because they should use their |
Adding a extra callback which returns a boolean is a last resort, because it makes the life of consumers of maquette harder. If maquette could do the logical/right thing automatically that would make things a lot easier/simpler. |
Maybe I'm getting ahead of myself. I was looking at the problem as "what could we add to Maquette that would provide a general solution for any of these situations in the future". I have some concerns implementing a specific solution for a feature that's still in draft, but I'd be happy to consider it. From your previous responses it sounds like you recommend identifying custom elements based on their element name and waiting to attach them to the DOM until after |
I am not sure what the benefits would be for non-custom-element vnodes to delay being added to the DOM. Without a There is one more concern about A-Frame. I like the objectives of the library a lot, but it seems like they have chosen the DOM to be their API. This leads to a lot of overhead when using maquette or React, because the following steps are taken:
This not only causes 3 extra representations of the data (vnode, DOM Node, three.js objects) to remain synchronized, it also causes an extra delay of 1 animationFrame :-(. I really like to see what it is like to create a 3d scene based on plain data and a pure render function. Coupling maquette with A-Frame would certainly be a good prototype for this, but I expect that a render function that draws directly to webgl would be the most performant. I hope that my analysis is not stopping you from taking this experiment further. I think that we should not burden maquette (size and performance) with my suggested approach for custom elements just yet. I think it is wise to wait for more people and more usecases. Please use your modified version of maquette for coupling with A-Frame and I am curious to see the results! |
I understand your concerns about A-Frame being less efficient than other options. I think that efficiency can be important, but often a larger barrier can be reaching a critical mass of developers to make a technology viable. By using the DOM it simplifies using and sharing components by leveraging the lingua franca of the web. As a contrast, ReactVR offers a solution that doesn't require custom elements. This is OK for React developers, but for other frameworks like Dojo, Vue.js, or Angular, which already know how to work w/ the DOM it adds an additional layer of complexity or forces a framework decision. If I were to make a framework with Maquette that rendered directly to a canvas it would be more efficient at the cost of less flexibility.
I'm happy to continue working on this as a demo and come back to this issue once we have some data points. So far we've identified two possible solutions
I'm still inclined to pursue the former solution because it has the least impact on Maquette and only a small amount on a user that is already using an advanced technology. I'll update this ticket when I have a working demo. P.S. If you haven't seen it yet, A-Frame inspector is a great tool to play around w/ A-Frame and export a scene. |
I've created a working demo at https://devpaul.github.io/derpymon/ (source: https://github.com/devpaul/derpymon). I modified maquette (devpaul@fd16bde) by adding a A flag was the most straight-forward way to solve this particular issue, but I also added a |
I looked at the example and the code. I think it looks good! I am still not convinced that there are usecases that would not be possible if element names which contain a dash were added to the DOM later. |
That's fair. It would be easy to introduce more functionality like the |
Hi Paul, we want to address this issue in maquette 3.0. |
I am trying to combine Maquette and A-Frame to render a WebVR scene and am running into an issue with intermediate DOM states.
When Maquette creates a new DOM node it attaches the new DOM node to the parent node before adding attributes and properties: https://github.com/AFASSoftware/maquette/blob/master/src/maquette.ts#L789-L795. This causes an issue with A-Frame which uses Custom Elements that watch for and react to changes in the DOM. When Maquette attaches the new DOM node to the parent, it is incomplete and thus creates an intermediate and invalid DOM.
These intermediate DOM states are problematic because they're not rendering what the developer expects and they prematurely trigger observers and proxies.
As a comparison, A-Frame resolves these issues w/ React by explicitly updating attributes in
componentDidUpdate()
: https://github.com/aframevr/aframe-react/blob/master/src/index.js#L91. But I don't think Maquette offers a similar mechanism to update the DOM before it's attached.I think the most straight-forward solution would be to call
initPropertiesAndChildren()
ondomNode
right before it's added to the DOM.The text was updated successfully, but these errors were encountered: