stimulusreflex / futurism

Lazy-load Rails partials via CableReady
MIT License
407 stars 19 forks source link

Include js-based embeds into posts #67

Closed staleo closed 3 years ago

staleo commented 4 years ago

Hi, thx for your efforts and progress this gem shows!

However, if I include some third-party embeds (tweet, telegram post, etc), it doesn't work. Youtube does work btw, but youtube embeds don't require any additional js to be loaded.

Example of tweet embed: <blockquote class="twitter-tweet"><p lang="en" dir="ltr">Apple enters the 5G market with new line of iPhones <a href="https://t.co/pcWqTWOy29">https://t.co/pcWqTWOy29</a></p>&mdash; Financial Times (@FT) <a href="https://twitter.com/FT/status/1316103751883726850?ref_src=twsrc%5Etfw">October 13, 2020</a></blockquote> <script async src="https://platform.twitter.com/widgets.js" charset="utf-8"></script>

Telegram embeds are way prettier (but still don't work because it's a js call) <script async src="https://telegram.org/js/telegram-widget.js?12" data-telegram-post="bloomberg/641" data-width="100%" data-userpic="false"></script>

Any ideas on how to make this all work? Best.

p.s. If this is more like a feature request (and I didn't design a post like a feature request), I'm sorry for that.

rickychilcott commented 4 years ago

Is it because they are async script tags? Can you try removing to see if it fires?

rickychilcott commented 4 years ago

Actually, check out the security section of https://developer.mozilla.org/en-US/docs/Web/API/Element/innerHTML

I’ll try to research or think about a workaround.

staleo commented 4 years ago

Hi Ricky, thank you for a quick response!

Is it because they are async script tags? Can you try removing to see if it fires?

No, removing async doesn't help, sadly.

Couple words on the workflow: people write posts, and sometimes they not only add text paragraphs, but also input links to external services, like youtube, twitter, etc. It'd be great to autoparse their tweet links, so I gsub their links to embeds (still not sure if it's better to do on frontend or on backend, upon the model save). That's pretty much it.

It would be crazily good if this flow and futurism could work alongside.

rickychilcott commented 4 years ago

I'm not sure I have a 100% clear answer but have some ideas to try.

https://ghinda.net/article/script-tags/ gives a good background on why this happens and ways around it in general

Futurism utilizes CableReady under the hood, specifically outerHtml and maybe in the future innerHtml operations. We could ask the CR team to see if they'd provide a execute_js parameter to these, but I'd like to see if we can avoid that for now. Searching CableReady issues doesn't come up with anything related to <script> tags.

While thinking, I wondered if morphdom (also used by CR) had anything to say about <script> tags, and it does have some functionality to execute script tags, triggered by this issue https://github.com/patrick-steele-idem/morphdom/issues/178 It looks like if we changed futurism to use CR morphs then we'd get this functionality, for free, but we're not using a morph we're using outerHtml.

Instead of capturing the futurism:appeared event, which doesn't give you the element(s) that have been acted upon, you could listen for the cable-ready:after-outer-html event and then execute the scripts yourself.

I'm not able to setup a test environment right now, but maybe this concept could work:

document.addEventListener('cable-ready:after-outer-html', (event) => {
  const changedElement = event.target

  Array.from(changedElement.querySelectorAll("script")).forEach(node => {
     const script = document.createElement('script')

     //copy over the attributes
     [...node.attributes].forEach( attr => { script.setAttribute(attr.nodeName ,attr.nodeValue) })
     script.innerHTML = node.innerHTML

     node.replaceWith(script)  
  }
})

Do note that a) there are likely syntax and logic errors :), b) I may be wrong about what is passed into the event listener, c) this may not work at all

Let us know what you come up with and we'll see if we can/want to make this more of a first-class citizen.

julianrubisch commented 3 years ago

Yeah morph with children_only: false could work, but we saw a couple of issues related to that im StimulusReflex, so it would require a significant amount of testing and could lead to a major version bump, when we're not even at 1.0 yet 😂

I'm wondering if we could use data.operations here

https://github.com/julianrubisch/futurism/blob/master/javascript/futurism_channel.js#L37

to iterate over the changed elements and pass them as event.detail, either to the existing event, or (preferably?) to a new one

rickychilcott commented 3 years ago

@staleo did you try my suggestion?

julianrubisch commented 3 years ago

I'm closing this due to inactivity. Feel free to comment here with more information and I'll reopen!