adamhaile
commented
6 years ago Thanks for writing this up so thoroughly. It's cool to see how people start using Surplus, both because they often follow patterns I hadn't thought of and also because it shows where there are shortcomings in the (still developing) docs I've put up.
Your intuition is right: the router is the core of the issue. page.js and your mount() command are imperatively replacing the page contents when the route changes. This is sometimes called an 'implicit state' problem: there's an implicit state of your app -- "what's the current route?" -- that isn't represented explicitly by any data signal. S only knows about changes to data signals. No data signals to track routing state means no changes, no changes means no lifecycle, no lifecycle means no S.cleanup() calls. (BTW, being removed from the DOM doesn't mean a node is destroyed: it's still present to Javascript.)
I just put up an example of routing in the new surplus-demos repo that may help you. You can see the code here or run it here. To keep it simple, it just uses hash-based routing.
The important bit there is that changes in the route are pushed into a data signal, hash() in the code. We then construct the view in a computation which watches hash(). That way, when hash() changes, the old view is torn down, thereby disposing any computations and running any cleanups they created.
If you want to stick with page.js, you can proxy its actions through a data signal. Something like:
// instead of `page` constructing our new view, it sets a data signal with a function that produces the new view
// then our `view()` computation watches the data signal and actually constructs the view
// that way, when we move to a new page, `view()` re-runs, disposing the old view
const viewFactory = S.data(() => <div/>),
view = S.root(() => S(() => viewFactory()())),
spage = (route, fn) => page(route, () => viewFactory(fn));
page.redirect("/", "/mirror");
spage("/mirror", () => <Mirror />);
spage("/todos", () => <TodoComponent />);
spage("*", () => <NotFound />);At that point, view() is a signal representing the current view, so you don't need any of your mount() or unmount() commands, just use Surplus:
getMountPoint().appendChild(<div id="app">{view()}</div>);
DorianGrey
Hi there,
I've recently been experimenting a bit with
surplusands-jsand just hit a problem with proper cleanup / lifecycle management. For demonstration purposes, I've created a small and quite dumb clock component that simply updates the displayed time by updating itself viasetInterval. As a result, this interval has to be cleaned up when the component gets destroyed. The source code of this clock can be found here: https://github.com/DorianGrey/surplus-ts-playground/blob/master/src/app/footer/Clock.tsx As you see, it has aS.cleanupdefinition at its bottom that should stop the interval when the computation gets cleaned up: https://github.com/DorianGrey/surplus-ts-playground/blob/fc2643419d8c52dc413aacda36b83945d0efac28/src/app/footer/Clock.tsx#L36-L38 This follows the examples and descriptions found on this repository and the one froms-js.Since the first component using the clock exists permanently, this cleanup should never be called. However, I was asking myself if it works in general, and decided to put another clock component on a different page to figure that out. The reproduction can be found on this branch: https://github.com/DorianGrey/surplus-ts-playground/tree/lifecycle-failure-repro (Simply clone +
npm i+npm start) This approach creates an ID per clock instance, and each update via the interval generates a warning to the browser console containing this ID. When accessinghttp://localhost:3000, the redirect tohttp://localhost:3000/mirrortriggers, the page that contains the second clock. There will be two warnings per second on the browser console, one per clock. This page - like all others - usesS.rootwhen creating the component layout. The clock, on the other hand, just usesS(...).Expected behavior
When moving to the
todospage via the link in the top-right corner, the clock on themirrorpage should be destroyed and cleaned up, thus the second clock should stop, while leaving the one in the footer alive and alone. I.e.:S.cleanupshould be called.Current behavior
The defined cleanup does not get called, thus the clock keeps running. When moving back to the
mirrorpage via the link in the header, another one gets created - prototype of a leak.Additional information
page.jsthat simply removes the page components from the DOM tree and discards its own reference afterwards when leaving a page. As a result, that component should no longer exist. See: https://github.com/DorianGrey/surplus-ts-playground/blob/master/src/app/routing.tsxS.rootinstead ofS(...)in the clock component with no luck.Please tell me if some additional information is required.