Coder Social home page Coder Social logo

forgo's People

Contributors

chronodave avatar jacob-ebey avatar jeswin avatar nianjie avatar spiffytech avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar

forgo's Issues

Component arrays rendered alongside siblings render as [object Object]

image

If I wrap the array in a parent element (e.g., as the only child inside a div) everything renders fine

import * as forgo from "forgo";

function Component() {
  return {
    render() {
      const vals = [1, 2, 3];
      return (
        <div>
          <header>Header</header>
          {vals.map((val) => (
            <p>{val}</p>
          ))}
        </div>
      );
    }
  };
}

function ready(fn: any) {
  if (document.readyState !== "loading") {
    fn();
  } else {
    document.addEventListener("DOMContentLoaded", fn);
  }
}
ready(() => {
  forgo.mount(<Component />, document.getElementById("root"));
});

Sandbox

Add support for dangerouslySetInnerHTML

This isn't really needed on the client side; one could always use innerHTML on the actual element. But necessary for server-side rendering where we don't have actual elements.

forgo.mount() typing not matching with behaviour

When trying to run the following code I got an unexpected error:

import * as forgo from "forgo";

const App= () =>
  new forgo.Component({
    render() {
      return <p>Tooltip</p>;
    }
  });

forgo.mount(document.getElementById("root"), <App />);

forgo.min.js:520 Uncaught Error: The container argument to the mount() function should be an HTML element.

Now, because of the typing of mount(), this doesn't throw an error, though I'm not sure why.

export function mount(
  forgoNode: ForgoNode,
  container: Element | string | null
): RenderResult {
  return forgoInstance.mount(forgoNode, container);
}

null however is never a valid container because if (parentElement) will always return false.

API change for rerender

Continuing from #28

Instead of asking the developer to import rerender and manually wire it with args.element, you could just pass this to the render callback as the third argument, so the developer doesn't need to know about args.element (all they care about is causing their component to rerender).

So the render callback would look like:

...
return {
    render(props, args, rerender) {   // rerender is already primed with args.element so the dev doesn't need to wire it themselves
...
rerender() // <- didn't need to pass args.element

So to achieve that you just need to wrap the rerender function sth like:


  // Get a new element by calling render on existing component.
        const newForgoNode = newComponentState.component.render(
          forgoElement.props,
          newComponentState.args,
          () => rerender(newComponentState.args.element)) // this third argument can now be called in userland by simply calling rerender() provided as the third argument of the callback
        );

Attribute Transformers

Initial discussion at #54

Parts of the discussion are copied below for context:

Initial proposal by @spiffytech

Forgo could allow libraries to implement this if component had a method to modify attributes before they're put onto the DOM. Something like:

function someFunc() {
  transformAttr(name, value, args) {
    if (name.startsWith('on')) {
      return autoRedraw(value, args);
    }
    return value;
  },
  render() {...}
}

Then you could imagine a library exposing this the same way forgo-state accepts a component and returns a component with modified lifecycle methods.

Pair this with wanting click handlers to run inside the component's error boundary and we've got two examples where modifying DOM node attributes is useful.

Determine strategy for making Forgo extensible without expanding framework core

There will be bountiful cases where an app wants some extended version of what Forgo does today, but since Forgo aims to be simple and small, we want to arrive at a stable core that doesn't need constant tinkering and expansion to support new use cases.

What's the best way to facilitate this?

forgo-state is a good example of a framework extension. It's effective at its job, but I don't think it's a repeatable pattern. It has a lot of knowledge about Forgo's internals, which is both cumbersome and a barrier for non-core developers to do something similar. A hypothetical forgo-mobx library seems intense to implement right now.

Some examples of ideas requiring more than Forgo does today:

  • Automatically await callbacks and rerender when they return
  • Allow arbitrary functions to be run inside a component's error boundary, so that the UI updates appropriately if a click handler explodes
  • Hooks-esque stuff. Not really the way React does them (no magic variables, don't-call-it-stateful function components, etc), but e.g., if I use a setInterval(), I have to add mount + unmount methods and wire up both of those, plus the place I actually use the interval, touching three separate places of code. Right now it's not possible to make that setup/teardown reusable, short of creating a whole wrapped component. If args supported something akin to event listeners, that'd be easy for libraries to build on.
  • I've got a Portal component I need to contribute, but right now it depends on forgo-state which isn't ideal. It's easy to do without forgo-state, except I like that forgo-state minimizes what gets rerendered in a call tree. Making that logic reusable would be great.
  • How could an app be offered a Context API?

A few things I'm thinking through:

  • How can we let reusable code tinker with a component's lifecycle methods? What does that look like? If we support that, do we still need the lifecycle methods? Are we even willing to consider major API changes in the first place, even if the end result is still small and simple?
    • If wrapping components is still the way forward, how can we make that not boilerplate-y and error-prone? I'd guess a complex component might want 5-10 hooks-y things.
    • Maybe make component wrapping feel more like using middleware?
  • If some code wants to hook into attrs (e.g., to wrap click handlers), how would Forgo support that? Technically today you could just wrap each of your handlers manually, but what if you wanted that applied across your whole project? With arbitrary opt-outs?
  • How can we make forgo's core less of a leaky abstraction? Can we e.g., make userland blind to implementation details like args.element?
  • What other stuff like forgo-state's call tree evaluation makes sense to make reusable?

Adding the afterRender() lifecycle method

Edit: The first proposal suggested getting rid of (/renaming) mount(). Retracted.

Currently, we have two lifecycle methods - mount and unmount.

unmount() is very clear, it happens when the component goes away (and the underlying node is removed from the DOM tree). It's a good place to write tear-down logic.

mount() - is a little tricky. Currently it gets called the first time a component is mounted on a node, and does not get called when it gets reattached to another node. Re-attach happens, for example when a CustomComponent renders a DIV tag in the first pass, and a P tag in the second pass. It is useful as a function that gets called only once for a component, and never afterwards. render() on the other hand, gets called multiple times.

For application developers, mount was never necessary. Whatever you could write in mount() could already be written inside the component constructor or within a component's render() method.

So, for people who want to detect when the DOM node changes, I propose adding a new method called afterRender() which fires just after the rendered components are attached to actual nodes. At that point, args.element would point to the updated DOM node.

@spiffytech @jacob-ebey Any thoughts?

Unconditionally updating element attrs causes side effects

Sandbox

Every time an <img>'s src attribute is set (even if unchanged), that image gets loaded again (load event is fired). If the image wasn't successfully loaded last time, the browser hits the network to try loading it again.

Because Forgo sets all attributes during every render, every render triggers the onload handler, and additionally hits the network if the image 404'd or similar.

In the sandbox you can see in the console after pressing buttons (2) or (3) that the image events fire based on attrs changes, regardless of whether Forgo is involved. Button (1) shows the 404 behavior in the network inspector.

Cannot conditionally render specific elements

I'm trying to render certain elements conditionally. With other frameworks I would render null like so:

render() {
  let error: string | null = null;

  return (
    <div>
      {error ? <p>Encountered an error: {error}</p> : null}
    </div>
  );
}

Forgo blows up when I try to do that. I'm handling it by rendering an empty div for the moment, but I'd like a way to omit elements without adding noise to my DOM.

Ref value is not accurate in nested mount

I was getting strange results from calling getBoundingClientRect() on a ref value after mounting, the element's height was always 0. However, when logging the value inside a timeout, it seems to return the actual height. Stranger still is this only happens in nested components.

Nested mount
image

import { createElement, mount } from 'forgo';

const List = () => {
  let ref = {};

  return {
    mount: () => {
      console.log(ref.value.getBoundingClientRect()); // 0
      setTimeout(() => console.log(ref.value.getBoundingClientRect()), 0); // 927
    },
    render: () => (
      <div style="overflow: auto; flex-grow: 1" ref={ref}>
        <div style="position: relative; min-height: 0">
          <div style="position: absolute">Hello world</div>
        </div>
      </div>
    )
  }
};

const App = () => {
  return {
    render: () => (
      <div style="display: flex; flex-direction: column; min-height: 100vh">
        <List />
      </div>
    )
  };
};

mount(<App />, document.getElementById('mount'));

"Flat" mount
image

import { createElement, mount } from 'forgo';

const App = () => {
  let ref = {};

  return {
    mount: () => {
      console.log(ref.value.getBoundingClientRect());
      setTimeout(() => console.log(ref.value.getBoundingClientRect()), 0);
    },
    render: () => (
      <div style="display: flex; flex-direction: column; min-height: 100vh">
        <div style="overflow: auto; flex-grow: 1" ref={ref}>
          <div style="position: relative; min-height: 0">
            <div style="position: absolute">Hello world</div>
          </div>
        </div>
      </div>
    )
  };
};

mount(<App />, document.getElementById('mount'));

Components are unmounted / never mounted if they return null

#37 got me curious to seek clarity on what Forgo does when a component returns null. I see that Forgo unmounts the component if it returns null, throwing away the component's state (sandbox).

This makes sense given the explanation that component state is stored on its DOM node, but is unexpected as an application developer. The decision not to display any HTML seems unrelated to the decision to discard component state. For example, if a component wants to return null until a resource has loaded that seems to not work right now.

If Forgo bases all its internal bookkeeping assuming a component always corresponds to a DOM node, does that mean this behavior would be pretty involved to change? I think even Fragments are implemented by selecting the first DOM node they contain, right?

What are your thoughts here?

Any ideas on how to approach a "context"?

Ok, I'm at the point that suspense is supported for SSR. This looks something like:

let retry = 1;
const app = <App />;
let promises: Set<Promise<any>> = new Set();
const onError = (err: any) => {
  if (err && err.then) {
    promises.add(err);
  }
};

let html = renderToString(app, { pretty: true, onError });

while (promises.size > 0 && retry <= maxRetries) {
  console.log("RETRY", retry);
  console.log("PROMISES", promises);
  await Promise.all(Array.from(promises));
  retry++;
  promises = new Set();
  html = renderToString(app, { pretty: true, onError });
}

Now the question is how should we approach context? The specific use-case here is storeing data that was loaded via suspense on server in a global for the client to consume.

Any thoughts here?

Components are double-rendering elements

I'm creating a form that adds/removes fields based on a radio selection. The initial render is fine, but changing the radio value causes the radio buttons/labels to double-render and one of the form fields to double-render. I can't spot any mistakes in my component code that would explain this behavior.

Here's a sandbox demonstrating the issue.

Checklist for The Big Update

Stuff we need to do before we're ready for our "make some noise" announcement. @jeswin, feel free to edit this.

Probably critical

  • #59
  • #46
    • We know this will be an ongoing project, but we want to get some wins ahead of the release.
  • #50
  • #62

Nice to have

  • #39
    • Having problems returning null from render() feels weird coming from other frameworks. It'd be nice to knock this out before drawing attention to Forgo.
  • #58
    • Marking this because it minimizes "what-about"-ism from prospective users
  • Making the error boundary general-purpose

Incorrect procedure during component root element tag change

Sandbox with forgo-state
Sandbox without forgo-state

I think this may be what triggered of forgo/forgo-state forgojs/forgo-state#2; this bug was introduced in the same commit mentioned there, and now knowing the more specific cause I can reproduce this bug even after [email protected].

When a component renders and returns a different root tag than before, the old tag gets marked for unloading but never actually gets cleaned up. So every time the component switches tags, the parentElement's list of unloadable nodes grows longer an longer.

The next time the parent rerenders, the node gets unmounted, which prompts forgo-state to unbind it from the state. Except the component never gets remounted, so the new element never gets bound back to the state, and it just becomes unresponsive to state changes, while the parent's unloadable nodes list grows without bound.

Because the new tag got rendered, the UI looks like it's correct until you try interacting with the updated element. And if you're not using forgo-state, you might not even notice, since the component holding the new tag can correctly ask itself to rerender. The deletedNodes/unmount part happens even without forgo-state, but things seem like they're working fine as long as the component doesn't have an unmount method.

I'm investigating how to fix this, with these goals:

  • Prevent the unloadable nodes list from growing unboundedly
  • Stop unmounting the component just because its tag changed

Runtime-modifiable lifecycle events

Initial discussion at #54

Initial proposal by @spiffytech

function MyComponent() {
  return (component: ComponentBuilder) => {
    component.mount(() => {
      const interval = setInterval(doStuff, 1_000);
      component.unmount(() => clearInterval(interval));
      const socket = makeWebsocket();
      component.unmount(() => socket.close());
    });
    component.render(() => <div>Hello world</div>);
  };
}

This style feels good to me. What do we get from nesting the arrow function within MyComponent?

This style starts to feel a bit like SolidJS, which might be a good direction to go. Their approach seems like a logical evolution of what major frameworks have turned into.

I like this style more than trying to add events on top of the existing monolith object API.

We might need to make some multicast and some not and eat that as a documentation issue. transformAttrs would be the same as render - no point in having two of them, you'd really want (manually?) composed functions instead.

Consuming projects can't find `JSX.IntrinsicElements` after v2.1.2

In v2.1.2 consuming projects can't find the JSX.IntrinsicElements interface. forgo is exporting JSX, but forgo.createElement isn't detecting that.

This was introduced by the merge of 812177f (my commit, my bad). I'm not sure why this didn't turn up when I tested of that commit.

I'll have a PR up once I identify a fix.

Prior to v2.1.2, forgo declared namespace createElement with namespace JSX inside of it. 812177f switched the full module import (import "./jsxTypes") to a types import (import type "./jsxTypes") to fix an esbuild issue, and that means the old way of declaring the createElement namespace doesn't work.

I'm trying to determine a new way to do that, and also why that's necessary in the first place. I can't find any documentation indicating TS JSX needs a createElement namespace. So presumably there's another way to go about the problem.

./dist/index.js does not exist

When working with eslint-plugin-import I noticed that my forgo import didn't seem to want to resolve. When inspecting why, it seems that the main field in package.json points to ./dist/index.js which does not exist when you download forgo via npm i forgo.

image

Is it possible to update the main field to ./dist/forgo.min.js? I'm not entirely sure if this breaks non-minified builds, however.

Fragment support

Already running into cases where fragment support is needed. A simple example is an "hoc" component where it just renders out the children property. Currently you have to wrap the children in another element like a div.

Handling async/event-driven renders?

How should I handle components that need to rerender when a promise resolves, or when an event fires? I'd figured I could set up my promise/event handler inside mount() and call rerender(), but in the context of #14 I now realize that the args.element provided to mount() will be stale by the time something tries to use it to rerender. The docs show some event handlers, but only ones used in the context of element references that are safe to recreate each render, and not situations where a handler should last the lifetime of the component.

Is there a recommended way to handle this?

Forgo not compatible with jsxFactory TSX constructor?

I tried using Forgo with ESBuild and Snowpack, and couldn't get them to work. As best as I can tell they don't support the TypeScript new react-jsx/jsxImportSource JSX constructor type, and I couldn't get Forgo to work with those bundlers using the classic jsxFactory setting. ESBuild seems to use its own custom TypeScript parser. Not sure what's up with Snowpack.

I don't know if this is a Forgo docs deficiency or a bug report, or if it's worth working on since I'll bet everything will support react-jsx within a year. And Parcel is okay for me right now. But with Forgo 1.0 looming, I figured I'd raise the compatibility gap for consideration.

Fragment usage

I'm trying to render a list as shown in the example but it assumes rendering a single element.

const list = [
  { key: 'a', value: '1' },
  { key: 'b', value: '2' }
];

const Example = () => new forgo.Component({
  render() {
    return list.map(item => [
      <p>{item.key}</p>,
      <p>{item.value}</p>
    ])
  }
});

In React, you could wrap listed items like this into a fragment which you can attach a key to.

return list.map(item => [
  <Fragment key={item.key}>
    {[
      <p>{item.key}</p>,
      <p>{item.value}</p>
    ]}
  </Fragment>
])

How would you do this in forgo?

Forgo benchmark results

I've submitted Forgo to the JS framework benchmark suite. Forgo's performance has been adequate for my own apps so far, but I'm not really pushing the envelope. Forgo doesn't need to be "the fastest gun in the West", but we want to make sure it's not the slowest, either.

The official results won't include Forgo until the PR gets accepted, but here's what I see running select benchmarks locally. All scores are mean-average milliseconds.

Forgo VanillaJS Mithril React Hooks Alpine
Insert 1,000 elements 170 84 121 120 285
Update every 10th row 1,100 187 278 216 239
Insert 10,000 rows 1,649 1,014 1,305 1,556 2,768

The tests discourage microoptimizing - they should reflect idiomatic usage of the framework.

  • VanillaJS is obviously our theoretical maximum performance.
  • Alpine is the slowest framework in several categories on the official results.
  • Mithril is our closest analog. Similar component model, similar scope, manual rerenders, etc. So it should be possible for us to hit their performance.
  • And React because why not ¯\_(ツ)_/¯

So anyways...

Forgo's on the slower side, with some room for improvement, especially for mutating existing elements.

Any thoughts on low-hanging fruit to target?

Here's the flamegraph for the Insert 10k test:
flamegraph

flamegraph close-up

I don't know how to tie a specific item in that graph to which component was being rendered (short of just stepping through the debugger until I've hit a function N times), but there's clearly a lot going on.

forgo.unmount() not called after removing element

The unmount() function does not call when unmounting a component. Sandbox

import * as forgo from "forgo";

const App = () => {
  const component = new forgo.Component({
    render() {
      return <p>Tooltip</p>;
    }
  });

  component.unmount(() => alert("unmounted!"));

  return component;
};

forgo.mount(<App />, document.getElementById("root"));

// forgo.mount(null, document.getElementById("root"));
document.body.removeChild(document.getElementById("root"));

I'm not entirely sure if either method correctly unmount a Forgo component.

Make a create-forgo-app tool

Most people use Create-React-App to build the scaffolding - and it's indeed quite useful to have one for Forgo. It can also set up some best practices.

Changing a component's root tag drops + recreates all children

Sandbox

While working on #47 I noticed that when a component's root tag changes, forgo drops and recreates all child nodes of the component. Anything stateful (inputs, web components, unmanaged nodes) will be wiped clean or lost.

We should copy all nodes over when creating a new node for a component, rather than trusting the render cycle to recreate everything. This procedure should blindly copy all children, to ensure we rehome any unmanaged nodes.

API idea

Hey guys,

Really minor suggestion on API style. Is there ever going to be a case where a developer needs to call rerender(args.element) without supplying args.element? If not, could the API just provide a primed rerender() which can be invoked without supplying args.element? You could provide it as a third argument to render, then we don't need to import rerender either.

i.e. your example:

import { rerender } from "forgo";

function SimpleTimer(initialProps) {
  let seconds = 0; // Just a regular variable, no hooks!

  return {
    render(props, args) {
      setTimeout(() => {
        seconds++;
        rerender(args.element); // rerender
      }, 1000);

      return (
        <div>
          {seconds} seconds have elapsed... {props.firstName}!
        </div>
      );
    },
  };
}

Becomes:

function SimpleTimer(initialProps) {
  let seconds = 0; // Just a regular variable, no hooks!

  return {
    render(props, args, rerender) {
      setTimeout(() => {
        seconds++;
        rerender(); 
      }, 1000);

      return (
        <div>
          {seconds} seconds have elapsed... {props.firstName}!
        </div>
      );
    },
  };
}

More tests are needed

The current test cases are mostly integration tests and are incomplete.

We need

  • (a) more tests
  • (b) a path to unit testing individual functions

(a) is more important as of now.

Contexts

See previous discussion at #54

Copying some parts of it below:

Initial Proposal by @spiffytech

How could an app be offered a Context API?

Jeswin said: So, would it suffice to do a regular import (of a file such as the following) into the component?

This is how I'm handling it now (typically with forgo-state so I don't have to track what has to rerender), but static exports only work if the use case is amenable to a singleton. If you need something reusable (a repeatable component hierarchy, or a reusable application pattern), current Forgo requires prop drilling. Context APIs allow context to be created at arbitrary points in the component hierarchy, rather than as app globals.

I think it's also suboptimal to make state an app-wide singleton as a default. Sometimes that turns out to be a mistake, and then it's a whole big thing to change how it works. If the default is to make a Context, then if you suddenly need two independent copies of some component hierarchy, that's no problem.

The one big hassle with Contexts is types. A component expects to be run with certain Context values available, and I'm not sure how to express that in a type-safe way, guaranteeing a component is only instantiated in the correct context.

Example use case: in my app, I have a big list of cards with buttons on them. Click a button and refetch updated data from the server. I don't want the user to click buttons too fast, because when the network request finishes the layout in the affected region will shift, so whats under their thumb could change as they're reaching to press it.

So I want to disable all buttons in the affected region while the request is in flight, plus 500ms afterwards. There are 3-4 levels of component in between the top of the region and all of the buttons, so prop drilling the disabled state + the disable function would be a pain. And once I implement this, I'd like it to be reusable across parts of my app.

I don't want to make this a global singleton, because I want to gray out disabled buttons, and graying out the whole UI would be confusing, and also the user may want to go click things that have nothing to do with the affected region while the request is in-flight.

With Contexts I could make a MagicButton component that watches the context, replace all my stock s, and nothing else has to change.

I don't think Forgo needs Contexts to come with magic auto-rerender behavior. Since rerendering an ancestor rerenders all descendants, descendants don't usually need to be reactive to Context state. And if they do, that's what forgo-state is for. It's just avoiding the prop drilling that's needed.

Jeswin said: I am in favor of coming up with a proposal and then adding this feature; unless you think we can avoid prop drilling with some other technique.

I can't think of an a way around it with the existing API. Everything comes down to, if you want an out-of-band way to do this, you still have to communicate which specific out-of-band instance (key, symbol, etc.) you want to reference. Which comes back down to prop drilling or asking the framework to do the drilling for you.

To toss out an alternative, instead of putting contexts into core, a library could implement the feature if forgo had an API to walk the component ancestry and a stable component identifier.

const contexts = new WeakMap<ChildNode,unknown>();
export function set(ref, key, value) {
  contexts.set(args, value);
}
export function lookup(args, key) {
  return contexts.get(args.element) ?? lookupContext(forgo.getParentRef(args, key))
}

function MyComponent(_props, args) {
  libcontext.set(args, 'foo', 123);
  const bar = libcontext.lookup(args, 'bar');
}

I don't like that this leaks args.element to userland. I'd prefer a stable opaque identifier like a Symbol. But then Forgo would have to bookkeep a Symbol<->ChildNode lookup.

I do like that removing implementation details from forgo-state already calls for an API to walk the component ancestry, and that this aligns with the vision of an extensible core, rather than putting another feature into core.

Btw, can this (your example of disabling buttons) be done with forgo-state?

Sorta. In the current form of my scenario, there are known, hardcoded regions that need to be managed, so I think forgo-state would work for the case in front of me (where Foo & Bar are each full of cards):

164844109-c641d5a2-45a8-4b69-b910-57904d562f9f

But if I wanted to independently affect dynamic regions it'd get trickier (where each Foo is still full of cards, but now there's a bunch of separate Foos):

164843897-02e020ab-aa5b-42ff-a97d-9689e706f7f6

forgo-state might do it using bindToStateProps, but only if the dynamic regions had obvious keys that were globally unique across my app. I'll bet a bindToStateProps approach would be okay-ish for many scenarios, but it amounts to the developer inventing an ad-hoc approximation of scoping rules which will break down in complex scenarios.

Improve error handling for malformed components

The type forgo.ForgoComponent does not cause a compile error if the component function returns a node:

function ShouldNotCompile(): forgo.ForgoComponent<forgo.ForgoElementProps> {
  return <div>Oh noooooooo</div>
}

^^ This compiles successfully, but at runtime Fargo throws an error because it can't find component.render.

I suggest the following:

  1. Update the error message for a missing render method to include the name of the component function (fn.name) that returned the malformed component. That would have saved me a bunch of trouble tracking down which code was the problem.
  2. Update the TS types to reject that return value if Forgo isn't going to support it.

Error from orphaned elements trying to render

I'm getting this error in a certain code path is my application:

The rerender() function was called on a node without a parent element.

I can't manage to create a minimal reproduction for this, and I've spent a bunch of hours trying to figure out what's going on here, even had a friend help debug the forgo source code tonight, but I haven't figured it out. I'm hoping you have an idea.

It seems that when my component rerenders and returns null, Forgo isn't correctly updating its bookkeeping. It holds onto the old detached div in args.element, which eventually makes the rerender throw an error. The component's unmount() method isn't called in this scenario, either.

The page visually renders fine, but I previously managed to get this error in an infinite loop that locked up page, and since I don't understand what's going wrong I can't just let it go.

I have a parent and a child that both depend on the same forgo-state state, and am using forgo-router.

The application flow that's happening looks like this: MyComponent onclick -> ajax -> update forgo-state -> MyComponent.render -> queueMicrotask(navigateTo('/app')), return null -> OtherComponent.render -> ajax => update forgo-state -> MyComponent.render

MyComponent shouldn't get that second render, because the page has navigated away. It should be unmounted. But because it wasn't, forgo-state tries to rerender MyComponent, but because it's holding onto the old div (even though the last render returns null) Forgo tries to rerender it and blows up.

(Why queueMicrotask? because forgo-router errors out if the very first render the app does includes an inline navigateTo() call because no components have corresponding DOM nodes yet).

If MyComponent, instead of returning null, returns something like <p>hello</p> I don't get the error. If I do the navigateTo() immediately in the render rather than in queueMicrotask, I don't get the error.

I see the error printed several times, if that means anything.

Top of render() in MyComponent:

    render() {
      if (
        authnstate.accessExpiresInHours === null ||
        authnstate.accessExpiresInHours > 0
      ) {
        queueMicrotask(() => navigateTo("/app"));
        //return <p>Redirecting...</p>;
        return null;
      }

The component is declared with bindToStates([authnstate], ...).

MyComponent lives under App, which is also bound to the same state. App contains the router:

             {...
              matchExactUrl(myComponentUrl, () => <MyComponent />) ||
             <p>Welcome!</p>}

Please let me know if I can provide any other info that'll help. I'm sorry that I can't trigger this in a minimal sandbox, only in my full application.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.