christo-pr / dangerously-set-html-content

Render raw html at your own risk! v2
60 stars 8 forks source link

Problems with rendering multiple scripts #2

Closed lymine1996 closed 1 year ago

lymine1996 commented 2 years ago

Hi, some strange things happened when I was using this InnerHTML component. When I tried to include a script from a remote source (with the src attribute) along with another script written by myself, it returned a ReferenceError implying the function from the src script was not being recognized in the script I added. To be more specific, I was implementing a map in my HTML page using Mapbox service and got this error: ReferenceError: mapboxgl is not defined, where the function mapboxgl already exists in the src script. Has anyone experienced the same issue or was I wrong about the use of this component to some extent?

AyrolesQ commented 2 years ago

Hi. I'm experiencing the exact same issue ! My first script tag uses the src attribute. My second script tag (script made by myself) uses an object from the first script but I keep getting a ReferenceError.

christo-pr commented 2 years ago

I'll look into, my first though is that probably the scripts are not been executed in the correct order maybe, can you try to use defer on the script that uses the object and maybe that way it waits until the object is defined.

lymine1996 commented 2 years ago

Thanks for the suggestion. I tried to add defer to my own script but it's still not working properly.

christo-pr commented 2 years ago

Regarding to this, I have some ideas and I'll try them, but basically we are using a Range object, and it creates a new document object where it executes the external code, so I'll check if there's a way to fix this

christo-pr commented 2 years ago

Just for the record, this seems to work:

export function App() {
  const script = `<script src="https://code.jquery.com/jquery-3.6.0.min.js" integrity="sha256-/xUj+3OJU5yExlq6GSYGSHk7tPXikynS7ogEvDej/m4=" crossorigin="anonymous"></script>`
  const html = `
    <script>
      $(document).on('click', function() {
        alert('test')
      })
    </script>
  `

  return (
    <div>
      <InnerHTML html={script} />
      <InnerHTML html={html} />
    </div>
  )
}

I'll try to see if there's a way to make this work with having both scripts on the same string, but this could be a workaround for now

lymine1996 commented 2 years ago

Appreciate it for digging into this. I tried to split my code into two parts, with each containing a script tag. However, it still didn't work with the same ReferenceError. The part of code that ran into this problem is a map component, which is pretty much similar to the example here: Display a map on a webpage

solomon23 commented 2 years ago

Just for the record, this seems to work:

I'll try to see if there's a way to make this work with having both scripts on the same string, but this could be a workaround for now

This doesn't work for me :(

nircelero commented 1 year ago

I am struggling with this too. Did anyone find a solution for that?

christo-pr commented 1 year ago

Same library? because we don't do anything fancy to be honest, so I'm thinking this is maybe due to a race condition issue, and that it's related with the external js that you guys are trying to execute more than with the library itself, so not sure what we can do to prevent it.

nircelero commented 1 year ago

I was using a different external script. I am pretty sure that the order of scripts loading is different than actual order in the html. I eventually used a custom whenAvailable function to work around this issue and it worked: https://stackoverflow.com/questions/8618464/how-to-wait-for-another-js-to-load-to-proceed-operation

jcuozzo commented 1 year ago

I ran into this issue too. This Stack Overflow answer explains that scripts added dynamically are forced to load asynchronously. It might not cover all cases, but I was able to modify the component from this package to force the synchronous behavior by waiting for each script to load before adding the next to the document.

christo-pr commented 1 year ago

Thanks @jcuozzo! I thinks your solution for this problem is the correct one, since again, this library is no doing anything fancy with the js that's passed in.

I'll close this issue 👍