Skip to content
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

returning null in a view #201

Closed
iamjohnlong opened this issue Aug 8, 2018 · 22 comments
Closed

returning null in a view #201

iamjohnlong opened this issue Aug 8, 2018 · 22 comments

Comments

@iamjohnlong
Copy link

Returning null throws errors, is there a way not to return a root node?

render(vm) {
    return vm.state.bool ? <div>foo</div> : null;
}
@iamjohnlong iamjohnlong changed the title returning null in view returning null in a view Aug 8, 2018
@leeoniya
Copy link
Member

leeoniya commented Aug 8, 2018

is there a way not to return a root node?

no. domvm's current architecture assumes a 1:1 vdom:dom relationship: every vnode has a representation in the dom and every view must return exactly one vnode. it's up to the parent whether a view is part of its children.

@l-cornelius-dol
Copy link
Collaborator

l-cornelius-dol commented Aug 8, 2018

FWIW, in my experience I've never found this to be a significant limitation; and I have some pretty sophisticated apps using DOMVM.

@leeoniya
Copy link
Member

leeoniya commented Aug 9, 2018

if you're interested in a view persisting after unmount, you can always use const vm = createView(View, data) + injectView(vm) instead of defineView(View, data). this way only the dom & vtrree go away without losing internal view state.

@iamjohnlong
Copy link
Author

I'm playing around with creating a component/view based router like React Router. If a path doesn't match I was hoping to return null in the view so I don't have an empty placeholder node.

It'd be nice to return null but I'll come up with a workaround.

@leeoniya
Copy link
Member

leeoniya commented Aug 9, 2018

in React, does returning null have equivalent behavior to the parent never including the component (it gets unmounted and destroyed (loses all state))?

@iamjohnlong
Copy link
Author

it gets unmounted and destroyed (loses all state)

I don't think it so. But I'd have to test it.

@iamjohnlong
Copy link
Author

Looks like it keeps state. https://codesandbox.io/s/pk4yq52pmj

@leeoniya
Copy link
Member

leeoniya commented Aug 9, 2018

i'm not sure it demonstrates maintaining state of unmounted components since you're just using passed-down props.

what i mean is if Test had its own this.state that incremented on every componentDidMount.

this does not work:

import React, { Component } from "react";

class Test extends Component {
  state = {
    count: 0
  }
  componentDidMount() {
    console.log(`I am mounted ${this.props.msg} ${this.state.count++}`);
  }
  componentWillUnmount() {
    console.log(`I unmounted ${this.props.msg}`);
  }
  render() {
    const { toggleRender } = this.props;
    return toggleRender ? <div>hi</div> : null;
  }
}

export default Test;

this means an unmounted component always destroys any state, including when it returns null (which looks equivalent to not including it in the parent). so even though it's in the vtree, React destroys its state, along with it DOM if null is returned, right?

if this is the case, it's confusing to me. i would expect component that i maintain in the vtree across renders to continue to live, even if its dom is destroyed and un-rendered.

@iamjohnlong
Copy link
Author

iamjohnlong commented Aug 9, 2018

I'm pretty sure it holds the state. https://codesandbox.io/s/pk4yq52pmj

yes, when the component unmounts it's state is destroyed. when the component returns null it doesn't unmount and retains its state.

@leeoniya
Copy link
Member

leeoniya commented Aug 9, 2018

ah, ok.

i'll give this some thought. it's not a quick fix, but i see how it's useful and better than create & inject, though i do wonder if that can be made into something similar without modifying the core.

i don't see anything in https://reacttraining.com/react-router/web/example/basic that can't be replicated with a bit of js and domvm. this "maintaining view state despite unmounting in a declarative way" seems to be the thorny part.

would be interesting to do a proof of concept with createView, injectView and vm memoization to simulate this. it's certainly possible, though the ergonomics could be anywhere on the scale from not-too-bad to wtf.

@tropperstyle
Copy link

Just another perspective:

I often have sub-views whose life-cycle is best managed declaratively.

Those sub-views then need to pull data once they are initialized, and it doesn't make sense to render the content until the data is fetched, except for maybe a loading indicator.

Here's what I currently do:

function SomeView(vm) {
  let data;

  vm.config({
    didMount() {
      fetch('/remote/data').then(_data => {
        data = _data;
        vm.redraw();
      });
    }
  });

  return function() {
    if (!data) return el('div.some-view');

    return el('div.some-view', [
      // view content
    ])
  }  
}
.some-view:empty { display: none }

I've often thought it would be nice if I could return other views directly and simply use the parent as a controller, or have a promise delayed mount similar to the unmount feature.

@iamjohnlong
Copy link
Author

fwiw we moved to React from mithril at my work and I've found being able to return null in a component a nice way to handle things.

@leeoniya
Copy link
Member

leeoniya commented Aug 9, 2018

I've often thought it would be nice if I could return other views directly

this is a far bigger undertaking than handling null returns, and more on par with [and related to] returning fragments (#195). honestly, i'm not sure if either of these features are worth the effort involved and a resulting more complex reconciler/codebase.

fwiw we moved to React from mithril at my work and I've found being able to return null in a component a nice way to handle things.

i agree that this would be a good quality of life improvement rather than creating/using a create/injectView manager. but again, i'd like to see what this solution would look like before committing to modifying the core.

@leeoniya
Copy link
Member

leeoniya commented Aug 10, 2018

@iamjohnlong am I correct in understanding that returning null is really only useful for a single level?

if i have App -> Component1 -> Component2 and Component 1 returns null, then Component2 and its state is completely lost. e.g.: https://codesandbox.io/s/lyzvk98w7l

if using domvm's imperative views, it's possible to retain view state at every level:

https://jsfiddle.net/oLxhp148/

var el = domvm.defineElement;
var vw = domvm.defineView;
var iv = domvm.injectView;
var cv = domvm.createView;

var showB = true;
var showC = true;

function ViewA() {
  var i = 0;
  var vmB = cv(ViewB);

  return () =>
    el("div", [
      "ViewA " + i++,
      showB && iv(vmB),
    ]);
}

function ViewB() {
  var i = 0;
  var vmC = cv(ViewC);

  return () =>
    el("div", [
      "ViewB " + i++,
      showC && iv(vmC),
    ]);
}

function ViewC() {
  var i = 0;

  return () =>
    el("div", "ViewC " + i++);
}

var vmA = cv(ViewA).mount(document.body);

setTimeout(() => {
  showB = false;
  vmA.redraw();
}, 2000);

setTimeout(() => {
  showB = true;
  showC = false;
  vmA.redraw();
}, 4000);

setTimeout(() => {
  showC = true;
  vmA.redraw();
}, 6000);

@iamjohnlong
Copy link
Author

am I correct in understanding that returning null is really only useful for a single level?

Yes I believe so.

if using domvm's imperative views, it's possible to retain view state at every level

Rad. It doesn't have anything to do with returning null from a component but it's a nice pattern.

@leeoniya
Copy link
Member

It doesn't have anything to do with returning null from a component but it's a nice pattern.

Allow me to disagree.

DISCLAIMER: I'm not a React user, so plz correct me where i'm wrong.

The only method React provides of retaining local component state across mounts/unmounts is by keeping that component in the vtree. There is no possibility of declaratively excluding it from the parent to temporarily unmount it. This makes returning null a key part of React's architecture, and not simply a convenience. Thus, when looking at a parent full of child components, there's no way of knowing which ones will actually exist in the dom, without also knowing what each will be returning from its render().

Currently, domvm is built on a simple principle: if it's in the vtree, it's in the dom; if it's not in the vtree, it's not in the dom. Additionally, domvm provides the ability to retain local component state across mounts/unmounts beyond that of React without having to keep things in the vtree simply for retention purposes. I believe that domvm's retention facilities are both clearer and more powerful than what React provides (as demonstrated in the above fiddle).

Certainly, I'm not interested in implementing things simply because React does them and they're useful (or necessary) in React-land with its particular limitations / design choices / quirks.

Back in the context of routing. In React, if returning null is only useful for retaining a single level of local component state, then what does that mean for nested routes? Is all of their state retained in the parents? In a global store? In the router? Do all components become functional and stateless?

Sorry for turning a seemingly simple thing into a philosophical rant 😬

@iamjohnlong
Copy link
Author

Certainly, I'm not interested in implementing things simply because React does them and they're useful (or necessary) in React-land with its particular limitations / design choices / quirks.

I'm not asking for that. If the null return doesn't make sense for domvm then I'm ok with that.

As far as your other questions I'm not sure. There are several ways to handle state in react and I'm not going to try to explain them here.

@leeoniya
Copy link
Member

I'm not asking for that. If the null return doesn't make sense for domvm then I'm ok with that.

my hope is that this discussion yields some use-cases with a clear benefit to returning null - where the alternative is really ugly or counter-intuitive. if the only thing that null returns would provide is to allow sub-components to handle their own mounting/unmounting logic w/state retention (but only 1 level deep), then this would not be a good fit for domvm.

@iamjohnlong
Copy link
Author

iamjohnlong commented Aug 13, 2018

Yeah I'm not sure it would make sense to allow multi level state retention and where/how that would be handled?

@leeoniya
Copy link
Member

Yeah I'm not sure it would make sense to allow multi level state retention and where/how that would be handled?

one use case that comes to mind is nested tabs in an admin interface, each with local state. possibly with nested routes.

I see only two use cases for null returns:

  • temporary unmounting with local state retention (1 level)
  • mounting condition moved from parent into component

To be honest, neither of these is compelling enough to warrant the addition (which is not a one-liner, and needs handling added in multiple places in the codebase). The fact that state can already be retained by existing and uniform createView/injectView semantics without any caveats like "1 level" makes the first case moot to me. The second case is just code shuffling and IMO is worse and less obvious than excluding the component from its parent.

So it's not a matter of when multi-level local state retention could be useful, but rather what benefit null returns would provide over the existing api.

@l-cornelius-dol
Copy link
Collaborator

I personally prefer the createView/injectView explicit semantics as well. I would find returning null to achieve that very "magical".

@leeoniya
Copy link
Member

leeoniya commented Oct 4, 2018

probably the most ergonomic way to achieve a near-equivalent of a null return is to return a comment node for the view's root. this means that the view is effectively unmounted and out of the dom, but is still retained by domvm:

var el = domvm.defineElement;
var cm = domvm.defineComment;
var vw = domvm.defineView;
var cv = domvm.createView;

var vis = true;

function AppView(vm) {
  return function() {
    return el("#app", [
      vw(SubView) 
    ]);
  };
}

function SubView() {
  var i = 0;
  
  setInterval(function() {
    i++;
  }, 200);
  
  return function() {
    return vis ? el("div", "abc " + i) : cm("SubView " + i);
  };
}

var vm = cv(AppView).mount(document.body);

setTimeout(function() {
  vis = false;
  vm.redraw();
}, 3000);

setTimeout(function() {
  vis = true;
  vm.redraw();
}, 6000);

@leeoniya leeoniya closed this as completed Oct 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants