fix: vNext fast-router templates not attaching on safari 16+

[KingOfTac]

🐛 Bug Report

There appears to be an issue with the latest version of webkit that is causing Router to not get a template. Currently not aware if this is an issue with the vCurrent version of Router.

💻 Repro or Code Sample

Working on a repro.

Simple repro steps would be to create a new project using the latest FAST prerelease versions and add router with a few routes and a default layout.

🤔 Expected Behavior

Apps that use Router should be loading properly

😯 Current Behavior

After safari 16 was released, Router no longer gets a template attached either on initial page load or on route changes. Other FAST components built using the vNext libraries function normally.

💁 Possible Solution

Needs investigation

🌍 Your Environment

Safari on IOS 16.1
Epiphany Browser using webkit 16 on WSL Ubuntu-latest in windows 11

[KingOfTac]

@EisenbergEffect I think I discovered the issue here, but not sure if it is an indication of a larger issue.

I noticed in my debugging that the DefaultRouter's config setter was not getting called in safari16. In the web component template that hosts the router element, I am binding the route config through a property binding: <fast-router :config=${x => x.config}>. I moved the setting of Router's config to the parent component's connectedCallback and things started working normally. What is really confusing is that the issue is only happening in Safari16+, all other browsers including Safari<16 are working fine with setting the router's config through a property binding in the template.

[EisenbergEffect]

That's very strange. Can you isolate the issue to a simple component without the router?

[KingOfTac]

That's very strange. Can you isolate the issue to a simple component without the router?

I've been working on isolating, but no luck yet. It may also be a combination of things since I am using the inject decorator to inject the router config into the router's parent component.

[KingOfTac]

@EisenbergEffect I haven't been able to create a reliable repro yet, but seems like something changed in webkit16+ that is causing DI to break in some scenarios. I no longer think that this is an issue with Router, and instead it is in DI. I also don't think it is an issue with FAST at all but a bug introduced in Webkit since prior to 16 everything worked fine.

[bigopon]

Recently Aurelia has also been hit by some issues from webkit 16+. The issue is at aurelia/framework#1003

There's a breaking change in the spec at whatwg/dom#754, @jded76 and I have been trying to get a spec revert, and are waiting for some responses. You can join the discussion at whatwg/dom#819 (comment)

[EisenbergEffect]

@bigopon and @KingOfTac Thanks for helping to look into this. I have a couple of questions:

Is the primary issue that document.adoptNode(...) doesn't return the adopted node anymore? If so, could that code just be changed to use the original template.content?

If not, does using document.importNode(...) solve the problem universally?

If not, is there an issue with the View moving elements back to the fragment when a view is removed?

I'm trying to figure out if it's one issue or a combination of issues. I'll need to make these fixes in both fast-element 1.0 and 2.0, so I'm trying to do the simplest one-time fix possible.

Any additional information you have is helpful. Again, thanks for looking into this!

[jded76]

Is the primary issue that document.adoptNode(...) doesn't return the adopted node anymore? If so, could that code just be changed to use the original template.content?

It returns a node as usual, but this node doesn't have an owner document (it's still an empty document) because adoptNode returns early.

[KingOfTac]

The primary issue I have been facing is that dependencies injected into a component using either the inject decorator or the dependency as a decorator itself, are not available in the template for bindings on the initial render anymore. It sounds like the DI issue may just be a side effect of the above though it's just speculation at this point.

[bigopon]

The change in the behavior can be roughly summarized via the following pseudo code

document.adoptNode = function(node) {
  
+  if (node is Fragment && node.host) {
+    return
+  }
}

[EisenbergEffect]

Ew. Thanks @bigopon. That's definitely a problem.

[EisenbergEffect]

I started by trying to reproduce this with the Todo sample app. That app seems to work fine. I added some conditional template logic to see if adding/removing views dynamically was an issue. I can't seem to reproduce it with 16.1 (18614.2.9.1.12).

I'll try this with the router next. Seems like if it's a core problem it should be reproducible with basic templating though.

[KingOfTac]

I started by trying to reproduce this with the Todo sample app. That app seems to work fine. I added some conditional template logic to see if adding/removing views dynamically was an issue. I can't seem to reproduce it with 16.1 (18614.2.9.1.12).

I'll try this with the router next. Seems like if it's a core problem it should be reproducible with basic templating though.

I'll be able to circle back to creating a repro this evening, but in the meantime these are the rough steps that I think will repro with Router.

1. Create a router config extending RouterConfiguration
2. Create an App using the @customElement decorator with fast-router in the template.
3. Inject a DI container and the router config as dependencies.
4. In the connectedCallback, before calling super.connectedCallback register the default route recognizer with the container as a singleton.
5. In the template pass the config to fast-router via a property binding :config=${x => x.routerConfig}

In all other browsers and Webkit <16 this pattern was working perfectly on the latest prerelease versions of FAST. With Webkit16+ this stopped working and I had to introduce a workaround where instead of passing the router config to fast-router through a property binding in the template, I now have to get a ref to the router element and pass the config after calling super.connectedCallback.

I have also observed this happening in other DI scenarios where you want to bind properties from an injected dependency directly into the component's template.

I haven't tested it yet, but I suspect that constructor injection will show similar behavior. I'm not privy to exactly how and when DI resolution and template binding happens, but my gut feeling is something isn't happening at the same time it was prior to Webkit 16

[EisenbergEffect]

Template binding happens in the connectedCallback of FASTElement. The DI resolution happens "lazily" when the injected property is first accessed. So, that should happen while the template is being first rendered. I'm starting to wonder...what if Safari is invoking the connectedCallback before the parent property is set. That would cause DI resolution to fail...

[EisenbergEffect]

Or rather, what if something is going wrong with events fired during the connectedCallback. The Context protocol that DI is based on uses an event to find the container. If that event is not propagating properly, then the container location will fail.

[EisenbergEffect]

You could take DI out of the picture and just use straight context to see if that works.

[KingOfTac]

You could take DI out of the picture and just use straight context to see if that works.

I'm pretty sure this does work. I have a few services that are just using context and injected into components and it all works. It's the DI portion that fails.

[EisenbergEffect]

Just going bit by bit. I updated my todo test to use DI instead of Context and it all works fine. So, next I'll add the router.