intercom / intercom-rails

The easiest way to install Intercom in a Rails app.
https://developers.intercom.io/reference
MIT License
280 stars 106 forks source link

Compatibility with hotwired/turbo-rails #336

Open hcurotta opened 3 years ago

hcurotta commented 3 years ago

I started migrating a project from Turbolinks to the Turbo project and hit some issues. I have a workaround which might be useful to include in the documentation, or at least have here for others who encounter the same problem. Let me know if it would be helpful to PR somewhere.

Turbo feels like a drop-in replacement but it doesn't profess to being backwards compatible as DHH outlines: https://github.com/hotwired/turbo-rails/issues/67

Although Turbo follows the same patterns as Turbolinks, and has the same events, i.e. before-cache, visit, load etc. they are now namespaced under turbo instead of turbolinks, so turbolinks:load is now turbo:load.

The IntercomJS client that gets loaded from Intercom has a bunch of Turbolinks event listeners, as you can see if you view the file and ctrl-f for Turbolinks. Due to the new names these event listeners are ignored and Intercom throws the following error when navigating via Turbo.

image

I've messaged Intercom support and asked them to consider including the new event names alongside the current ones. In the meantime I've found the following workaround:

document.addEventListener('turbo:before-cache', () => {
  document.dispatchEvent(new Event('turbolinks:before-cache'))
});
document.addEventListener('turbo:load', () => {
  document.dispatchEvent(new Event('turbolinks:load'))
});
document.addEventListener('turbo:visit', () => {
  document.dispatchEvent(new Event('turbolinks:visit'))
});

Version info

Expected behavior

Intercom works when using Turbolinks to navigate

Actual behavior

Intercom JS throws an error

Steps to reproduce

  1. Install Turbo
  2. Load page
  3. Click link to another page

Logs

VM6567 frame-modern.412e5163.js:1 Uncaught TypeError: Cannot read property 'document' of null
    at o (VM6567 frame-modern.412e5163.js:1)
    at Object.read (VM6567 frame-modern.412e5163.js:1)
    at VM6567 frame-modern.412e5163.js:1
    at session_Session.createOrUpdateUser (VM6567 frame-modern.412e5163.js:1)
    at app_App.createOrUpdateUser (VM6567 frame-modern.412e5163.js:1)
    at Object.update (VM6567 frame-modern.412e5163.js:1)
    at VM6567 frame-modern.412e5163.js:1
    at <anonymous>:3:473
    at <anonymous>:3:910
    at replaceElementWithElement (turbo.es2017-esm.js:3925)
ghiculescu commented 3 years ago

This seems to have been fixed. The Intercom script now includes Turbo events:

image

And we've stopped seeing this error at some point in the last few days.

hcurotta commented 3 years ago

Oh that's great! Thanks letting me know. Intercom support were helpful and receptive but I didn't expect to see it incorporated this quickly.

Petercopter commented 3 years ago

I'm doing the same thing, migrating to Hotwire from Turbolinks. For forms that fail validation, we used to just render with a 200 OK. But now, we need to render with a 422 Unprocessable Entity. https://github.com/hotwired/turbo-rails/issues/34#issuecomment-754628629

When I get the 422, I get this error:

VM1314 frame-modern.4decb80c.js:1 Uncaught TypeError: Cannot read property 'document' of null
    at s (VM1314 frame-modern.4decb80c.js:1)
    at Object.read (VM1314 frame-modern.4decb80c.js:1)
    at VM1314 frame-modern.4decb80c.js:1
    at session_Session.createOrUpdateUser (VM1314 frame-modern.4decb80c.js:1)
    at app_App.createOrUpdateUser (VM1314 frame-modern.4decb80c.js:1)
    at Object.update (VM1314 frame-modern.4decb80c.js:1)
    at VM1314 frame-modern.4decb80c.js:1
    at <anonymous>:3:213
    at <anonymous>:3:650
    at replaceElementWithElement (turbo.es2017-esm.js:1476)

It looks like it's coming from Turbo, but the Intercom widget disappears as well. Is anyone else seeing this?

mrhead commented 3 years ago

@Petercopter, that error is coming from the Intercom widget. For 422 responses, I see an error coming from frame.134abfc8.js. I'll probably open a ticket for them.

mrhead commented 3 years ago

The Intercom widget listens to turbo:load, turbo:visit, and turbo:before-cache events.

Turbo doesn't fire any of these events during unsuccessful form submission, so the widget can't prepare itself for the DOM change.

I've already opened a ticket for Intercom so they can improve it, but in the meantime I've came up with the following workaround:

document.addEventListener("turbo:submit-end", (event) => {
  window.turboFormSubmissionFailed = !event.detail.success

  if(window.turboFormSubmissionFailed) {
    document.dispatchEvent(new Event('turbo:visit'))
    document.dispatchEvent(new Event('turbo:before-cache'))
  }
})
document.addEventListener("turbo:render", () => {
  if(window.turboFormSubmissionFailed) {
    document.dispatchEvent(new Event('turbo:load'))
    window.turboFormSubmissionFailed = false
  }
})

This code emulates the page visit flow after unsuccessful form submission, and it fixes issues with the widget.

tomdevery commented 2 years ago

APP ID: 15540 User/Lead (who reported issue): 5622a0411cf1a9784902c642 Can Impersonate?: No Conversation with Customer: https://intercomrades.intercom.com/a/apps/tx2p130c/inbox/inbox/4697146/conversations/40612403733

summera commented 1 year ago

@mrhead thanks for all the details! Do you know if this was ever resolved?

mrhead commented 1 year ago

@summera It looks like this was never fixed. When I remove our workaround, the Intercom widget fails on unsuccessful form submission.

summera commented 1 year ago

@mrhead thanks for checking! Apparently this is a deeper issue that others have brought up in the Turbo repo:

I wonder if there's a way to gracefully shut down and restart the Intercom widget without simulating the Turbo events. Any idea what the Intercom JS script is doing when it receives the turbo:load event? Maybe the same code could be executed in your workaround instead of firing off Turbo events?

hirowatari commented 10 months ago

Still an issue after two years. Any update from maintainers?

arutinn commented 6 months ago

I can confirm that this is still an issue.