Alpine: data-turbolinks-permanent: Uncaught ReferenceError on first navigation

[bep]

This is tested with v2.2.3 -- but this isn't a new problem.

This is basically #309 -- but I think I have narrowed it down a little to make it easier to grasp.

See this commit: https://github.com/bep/hugo-alpine-test/commit/4860d5fbde7a25133e9df94f5d9ae3c3e8f02543#diff-32861fdd10fb962832bc30955219001bR31

Here I "load" the data in the init() function.

There is a running version at https://agitated-kilby-da786e.netlify.com/

If you:

• Navigate to https://agitated-kilby-da786e.netlify.com/ -- the dropdown is populated OK.
• Click on one of the links down on the page (powered by Turbolinks 5.2.0) you get the error below. The dropdown is still populated OK.
• If you continue to navigate there is no more error.

VM141:3 Uncaught ReferenceError: item is not defined
    at eval (eval at saferEval (alpine.js:109), <anonymous>:3:52)
    at saferEval (alpine.js:109)
    at Component.evaluateReturnExpression (alpine.js:1515)
    at handleAttributeBindingDirective (alpine.js:533)
    at alpine.js:1473
    at Array.forEach (<anonymous>)
    at Component.resolveBoundAttributes (alpine.js:1459)
    at Component.initializeElement (alpine.js:1396)
    at alpine.js:1377
    at alpine.js:1369
eval @ VM141:3
saferEval @ alpine.js:109
evaluateReturnExpression @ alpine.js:1515
handleAttributeBindingDirective @ alpine.js:533
(anonymous) @ alpine.js:1473
resolveBoundAttributes @ alpine.js:1459
initializeElement @ alpine.js:1396
(anonymous) @ alpine.js:1377
(anonymous) @ alpine.js:1369
walk @ alpine.js:85
walk @ alpine.js:89
walk @ alpine.js:89
walk @ alpine.js:89
walkAndSkipNestedComponents @ alpine.js:1357
initializeElements @ alpine.js:1374
Component @ alpine.js:1304
initializeComponent @ alpine.js:1660
(anonymous) @ alpine.js:1650
(anonymous) @ alpine.js:1630
discoverUninitializedComponents @ alpine.js:1629
(anonymous) @ alpine.js:1649
(anonymous) @ alpine.js:1643
childList (async)
r @ turbolinks.js:5
o.relocateCurrentBodyPermanentElements @ turbolinks.js:5
o.replaceBody @ turbolinks.js:5
(anonymous) @ turbolinks.js:5
e.renderView @ turbolinks.js:5
o.render @ turbolinks.js:5
e.render @ turbolinks.js:5
t.renderSnapshot @ turbolinks.js:5
t.render @ turbolinks.js:5
r.render @ turbolinks.js:6
(anonymous) @ turbolinks.js:5
(anonymous) @ turbolinks.js:5
requestAnimationFrame (async)
r.render @ turbolinks.js:5
r.loadResponse @ turbolinks.js:5
r.visitRequestCompleted @ turbolinks.js:5
r.requestCompletedWithResponse @ turbolinks.js:5
(anonymous) @ turbolinks.js:5
r.endRequest @ turbolinks.js:5
r.requestLoaded @ turbolinks.js:5
(anonymous) @ turbolinks.js:5
load (async)
r.createXHR @ turbolinks.js:5
r @ turbolinks.js:5
r.issueRequest @ turbolinks.js:5
r.visitStarted @ turbolinks.js:5
r.start @ turbolinks.js:5
r.startVisit @ turbolinks.js:6
r.startVisitToLocationWithAction @ turbolinks.js:5
r.visitProposedToLocationWithAction @ turbolinks.js:5
r.visit @ turbolinks.js:5
r.clickBubbled @ turbolinks.js:6
(anonymous) @ turbolinks.js:5

[SimoTod]

Does it happen when you navigate to a different page or only when you navigate to the same page? @bep

[bep]

It happens in both those cases, but only the first navigation (I pushed a "home link" to the above site which should make it possible to trigger that) ...

[ryangjchandler]

Sounds like something going wrong in the mutation observer. @SimoTod I briefly remember you saying there were a couple of problems people were having with it - are there issues for those too?

[SimoTod]

I'll try to debug it over the weekend and see if I can get to the bottom of the issue. 👍

[bep]

I did a quick debug session -- and it fails for the 4th (there should be only 3, so there is a duplicate element coming in from somewhere) a href in the dropdown above.

[SimoTod]

Hi @bep, I finally had chance to have a look at your code.
I'm 90% sure that it's not an Alpine issue.

You have a x-for directive in a template tag which generates 3 items outside the tag.
You use turbolinks so, when you click on a link, your html is replaced by the one in your new page and alpine tries to initialise all the new components walking down your new DOM.

BUT

you also added data-turbolinks-permanent to your dropdown component which means that the DOM is cached and when you refresh the page, instead of being the original html for that component, turbolinks prints the html containing the generated tags outside the template tag.
Those elements have reference to item but the variable does not exist hence the error.

I assume you copied that html from an example since it doesn't look like you need to cache it.
In case you really need to do that, you have to make sure that you remove the markup generated by Alpine before it gets cached. You can listen for the turbolinks:before-cache event and clear the html in your handler.

for this specific example, removing data-turbolinks-permanent should solve the issue.
Let me know if it makes sense

[bep]

@SimoTod the example above is made up, but the use case is real. I really need data-turbolinks-permanent. The example above would possibly translate to a accordeon left side navigation. I want that state to be preserved when I navigate. If I somehow have to manually clear the DOM to make this work then AlpineJS isn't really working with Turbolinks (AlpineJS should at least provide a clear() function or something, but ...).

There is a simpler example in this repo:

https://github.com/alpinejs/alpine/blob/master/turbolinks-manual-test/index.html#L11

And the only reason that works now is that it's simpler.

[SimoTod]

Yeah, in my head this is an integration problem and should be either isolated as a generic issue fixed in an agnostic way or fixed in a bridge library (I'm not a big fan of https://github.com/alpinejs/alpine/blob/master/src/index.js#L16 either, it gives the impression that alpine is turbolinks-ready but as you pointed out it's not fully working). If tomorrow someone writes 'fastlinks' with different events, someone could argue that Alpine is 'broken' and requires more customisation in the core.

It's my personal view, not the official Alpine line though.

The only directive affected should be x-for but a clear function (or the thing i suggested initially) would lose the state of input field defined inside for element if they are not linked to a model so maybe it's not the best solution either.

[SimoTod]

@bep I drafted a PR which should remove the error and persist the state when using data-turbolinks-permanent.

Can you try https://cdn.jsdelivr.net/gh/simotod/alpine@bug/turbolinks-mutation-observer/dist/alpine.min.js and let me know if it behaves as you expect? Thanks

P.s. I still think that they should live in a separate library, something like alpine-turbolinks-bridge.js that you load alongside alpine. :)

[bep]

I tested your PR, and it removes the error situation, but it introduces other issues (errors) that does not involve data-turbolinks-permanent. This seems to be the most prominent error:

alpine.min.js:7 Uncaught TypeError: Cannot convert undefined or null to object
    at alpine.min.js:7
    at Proxy.forEach (<anonymous>)
    at y (alpine.min.js:7)
    at alpine.min.js:7
    at Array.forEach (<anonymous>)
    at ne.resolveBoundAttributes (alpine.min.js:7)
    at ne.updateElement (alpine.min.js:7)
    at alpine.min.js:7
    at alpine.min.js:7
    at e (alpine.min.js:7)
(anonymous) @ alpine.min.js:7
y @ alpine.min.js:7
(anonymous) @ alpine.min.js:7
resolveBoundAttributes @ alpine.min.js:7
updateElement @ alpine.min.js:7
(anonymous) @ alpine.min.js:7
(anonymous) @ alpine.min.js:7
e @ alpine.min.js:7
e @ alpine.min.js:7
e @ alpine.min.js:7
walkAndSkipNestedComponents @ alpine.min.js:7
updateElements @ alpine.min.js:7
(anonymous) @ alpine.min.js:7
y @ alpine.min.js:7
(anonymous) @ alpine.min.js:7
resolveBoundAttributes @ alpine.min.js:7
updateElement @ alpine.min.js:7
(anonymous) @ alpine.min.js:7
(anonymous) @ alpine.min.js:7
e @ alpine.min.js:7
e @ alpine.min.js:7
walkAndSkipNestedComponents @ alpine.min.js:7
updateElements @ alpine.min.js:7
(anonymous) @ alpine.min.js:7
(anonymous) @ alpine.min.js:7
setTimeout (async)
(anonymous) @ alpine.min.js:7
(anonymous) @ alpine.min.js:7
valueMutated @ alpine.min.js:7
set @ alpine.min.js:7
(anonymous) @ bundle.js:943
Promise.then (async)
search @ bundle.js:929
eval @ VM667:3
(anonymous) @ alpine.min.js:7
evaluateCommandExpression @ alpine.min.js:7
w @ alpine.min.js:7
l @ alpine.min.js:7
sendEvent @ bundle.js:88
search @ bundle.js:117
self.dispatch @ bundle.js:1018
eval @ VM666:3
(anonymous) @ alpine.min.js:7
evaluateCommandExpression @ alpine.min.js:7
w @ alpine.min.js:7
l @ alpine.min.js:7

Whether this should be fixed inside/outside of Alpine, I'm not sure -- but I think you would be surprised by the number of people who are looking for a lightweight alternative to Vue/React that is also going to be looking for an easy way to preserve navigational state. To me, AlpineJS isn't really interesting without Turbolinks.

[SimoTod]

It's just I don't like the design. Alpine shouldn't be coupled to turbolinks in its core.

You should have

• alpine.js, turbolinks.js,and alpine-turbolink-bridge.js
• alpine.js, awasonnewframework.js and anotherbridge.js
• and so on...

It would change very little but it will make more sense to me.

Btw, about the issue, can you post a codepen to reproduce it? Thanks

[bep]

Btw, about the issue, can you post a codepen to reproduce it? Thanks

I will try. Just a quick question: Do all of the existing Alpine tests pass with your patch?

alpine.js, turbolinks.js,and alpine-turbolink-bridge.js

Yes, you are probably right, but they need to be owned by this project (included in tests). I'm pretty confident that this project's success in large parts goes hand in hand with its Turbolinks (or similar) integration.

[SimoTod]

They seem to pass -> https://github.com/alpinejs/alpine/actions/runs/76041958

I've only changed stuff in the turbolinks listener and added another turbolinks listener so I don't think tests would catch any bug though.

Is ot the page you posted at the beginning of this thread? I can copy the html from there.

[ryangjchandler]

I don't think the success is largely down to it's usually good Turbolinks integration at all. It might play a part but it's definitely not the majority of it.

Also, separating the Turbolinks support to a bridge would also have it's own tests so it doesn't necessarily have to be under the core package.

[bep]

@ryangjchandler fair opinion. Note that I have done a fair amount of thinking/research on this subject. I even stated in some tweet that AlpineJS would a perfect match for Hugo (the project I'm running), but that statement was based on the assumed Turbolinks compatibility.

/cc @calebporzio

[ryangjchandler]

@bep Ah, you created Hugo. Love that project :)

I do think that there is fair reason for both approaches. From a maintenance point of view, keeping it in the core makes sense but from an opt-in and opt-out point of view, a separate "bridge" package makes sense too.

[bep]

Is ot the page you posted at the beginning of this thread? I can copy the html from there.

@SimoTod no, the failing stuff comes from a "real project" that I cannot share as-is for now. But the failing stuff is fairly standard x-for stuff, so I should be able to reproduce it in a simpler setup. I think. I will have a look at it tomorrow. And thanks for looking into this, vey much appreciated!

[SimoTod]

No worries, I'll try with some simple x-for to see how it goes. The change is on the listener so it shouldn't affect normal projects but I could be wrong.

Another possibility is that, since I branched off master which does contain a x-for refactoring, it could be the new code. I believe Caleb is finishing it up so it may be not ready for production as yet.

I'll apply the same fix to the latest tagged version and send you the link tomorrow so we can double check.

[SimoTod]

This is the same fix applied to the latest stable version. https://cdn.jsdelivr.net/gh/simotod/alpine@bug/turbolinks-mutation-observer-2/dist/alpine.min.js
if you want to try it out.

[bep]

This is the same fix applied to the latest stable version.

Yay! That last link works great! No errors!

[SimoTod]

Okay, so it could be the refactoring Caleb has been working on. I think we should still look into the issue and let Caleb know so he won't tag a broken release. Would you mind sending over an example of the error when you have time? No rush, thanks

[ryangjchandler]

@SimoTod did you want to move this issue across to the adapter repo?

[SimoTod]

I think it's better ti leave the issue in the Alpine repo, possibly mention that the adapter here and close the issue so, if people comes to alpine and search for turbolinks issues, they'll see the adapter, they can navigate to it and either use it if it fixes their problem or open a new issue if it's something new.

[HugoDF]

Closing since we've got a separate project to deal with Turbolinks integration: https://github.com/alpinejs/alpine-turbolinks-adapter

If you're having turbolink issues, do raise issues on the adapter repo https://github.com/alpinejs/alpine-turbolinks-adapter/issues.

[pspeter3]

I agree with @SimoTod that Alpine should consider removing the Turbolinks event listener. It definitely provides the impression that Alpine does not need an adapter. Instead people should use the alpine-turbolinks-adapter.