Fatal flaw with duplicate SPA config flow between renders

[bob2517]

Currently, when a node is removed from the DOM, it is scanned for inline acss tags, and that specific config is then removed.

Prior to new content being rendered, any inline acss tags in the new content gets added to the config.

This sounds fine in theory and works most of the time, but has the potential to go wrong when two sets of inline config contains the same data - like the same components, same events, etc. The removal of config written over always happens after the addition of the new config. And stuff can happen in between to mess things up.

Ideally, inline config that is about to be rendered should be loaded after the old config has been removed. Currently the old config isn't removed because it hasn't been rendered over yet and so it's still there when the new config is loaded. The removal happens during a mutation observer event, which happens at the end of the call stack, but that just compounds the problem. I can't load the new config in a mutation observer as I need it loaded before I render as part of the flow - it needs to be Zohan smooth with no delay and a mutation observer is going to postpone the render until after the callstack and spoil the rendering flow.

So only after the new config has loaded is the old config removed. But the problem is that all the events, rendering, etc. happens with two sets of duplicate config before the old config is removed.

In simple terms, I need to work out a way of removing old config within a container before it is removed, and only after that load the new config and finish the render.

It's not a problem on fresh page loads - this is just an SPA thing, and only to do with repeated config being loaded between SPA page renders.

I noticed this while fixing a hash SPA change. where it was re-rendering the underlying page and bringing in duplicate config, which shows up this problem. The only place on the live docs site when I've use a hash is on the image popup example, and for 2.5.1 I've brought it out of the code editor so it actually works with the popstate. This hash problem has probably been there since I released the last major change to inline config handling. Need more testers... another face-palm.

[bob2517]

Basically, that whole node mutation removal should come out of there, be placed in a separate function and called with the accs tag nodes that are about to be removed just before the new inline config gets loaded.

Hopefully there shouldn't be too much of a delay in render when I do this. There isn't a sensibler way to do it without splitting up the config into loaded segments, which is not something I'm keen to do even if it did sound like the best solution, which it doesn't.

[bob2517]

Devtools config changes and DOM clean-up can stay in node mutations and don't have anything to do with this issue. With devtools config changes the removal is already set up to happen before the addition within the mutation observer.

[bob2517]

The only problem I can see with removing the old config before adding the new config before the affected items get removed would be the invalidation of the lifecycle event for removing items, if that is used in the config. The lifecycle event would get removed from the config before the event is triggered.

That could be fixed by not removing the lifecycle disconnect event from the config until after the call stack is finished. Maybe - it depends on how the stack stacks the mutation. Would need to test it. That is the solution in theory though.

I've not even used that functionality yet - like doing something when an element is removed. I imagine people use it though.

That shouldn't be too hard a fix. I don't have an example for it currently, but I'll sort it out tomorrow. Hopefully that will be an end to this scenario.

[bob2517]

Meh - the delayed removal of disconnect won't solve it on its own. It could be tied to a conditional which may have just been removed. Plus there's still a possibility of duplicate config if I delay before removal. I'm going to have to put in a rule that if you want a lifecycle disconnect callback to happen, don't remove the config from the page. That's the simplest solution. I don't particularly like it, but I don't see another solution at the moment. I'd rather have a clean switch between inline config removal and loading than a messy inner core with config tied to parallel DOM elements.

[bob2517]

This is fixed offline, need to update docs with the proviso on using disconnectedCallback, and that event isn't actually documented yet so it shouldn't be a big deal.

[bob2517]

Closing for milestone tracking.